Add inherit_fds: so a loader can name an already-open input

[flavorjones]

Motivation / Background

I maintain HotCell, which runs Active Storage's image processing inside a container. The application hands that container its input as an already-open file descriptor, and caps what the container may write with RLIMIT_FSIZE.

Two of HotCell's operations use ImageProcessing::MiniMagick, and both have to copy that input out to a file first, because the pipeline names its source by path and a descriptor handed over a socket has none. The copy is a write, so the cap bounds what those two operations can read as well as what they write, and an input larger than the cap fails permanently before any processing happens. The libvips operations never had that ceiling, because they read the descriptor in place.

minimagick#605, released in 5.4.0, added inherit_fds: to MiniMagick::Shell#execute. It names IO objects in Process.spawn's redirect map, so the child inherits those descriptors and the command can be given /dev/fd/N in place of a filename. ImageProcessing::MiniMagick constructs the MiniMagick::Tool itself, so a caller has no way to reach that option.

Detail

This Pull Request adds inherit_fds: as a loader option and passes it to the tool at construction. The source stays a /dev/fd/N path, so every other loader option still applies to it.

File.open("input.tiff", "rb") do |file|
  ImageProcessing::MiniMagick
    .source("/dev/fd/#{file.fileno}")
    .loader(inherit_fds: [file], loader: "tiff", page: 0)
    .convert!("jpg")
end
# magick tiff:/dev/fd/3[0] -auto-orient output.jpg

mini_magick is a soft dependency and the Gemfile admits >= 4.9.5, so a caller can easily be on a version without inherit_fds:. Handing the option to one of those raises ArgumentError: unknown keyword: :inherit_fds from inside MiniMagick::Shell#execute, which tells the developer nothing about what to do. ImageProcessing::MiniMagick.convert_shim now checks the version first and raises LoadError, matching the LoadError this file already raises when mini_magick is missing altogether:

The `inherit_fds` loader option requires mini_magick 5.4.0 or newer, but mini_magick 5.3.3 is loaded. Please upgrade the gem.

inherit_fds: defaults to nil and is left out of the tool's arguments when it is not given, so nothing changes for existing callers and an older mini_magick is never handed the keyword.

The suite passes with no skips on mini_magick 5.4.0, 5.3.3 and 4.13.2. The descriptor tests run on 5.4.0 and newer; on anything older, the same pipeline is asserted to raise the upgrade error naming the version actually installed.

Additional information

The option name is mini_magick's, taken there from Process.spawn's "File Descriptor Inheritance".

This is the input side only. The output side cannot be handled the same way, because the pipeline picks its saver from the destination's extension and a /dev/fd path has none.

A pre-built MiniMagick::Tool source still ignores inherit_fds:, exactly as it already ignores loader, page and geometry. I left that alone rather than making inherit_fds: the one option that raises there. If you would rather that branch rejected loader options outright, that seems worth doing for all four at once, and I am happy to follow up.

/dev/fd is not available on Windows; the doc note says so.

[janko]

Thanks for that PR! I see you added tests, so it seems like it works. I'm just curious, how does the :inherit_fds option manage to apply when convert_shim gets called without a block? I thought that in that case MiniMagick::Shell#execute doesn't get called yet 🤔

[flavorjones]

@janko Thanks for taking a look.

You're right that MiniMagick::Shell#execute doesn't get called yet. Without a block, convert_shim returns the MiniMagick::Tool instead of calling it, and the option applies later: Tool#initialize stores it in @options (tool.rb:55-58), and Tool#call merges @options back in before Shell#run passes them to #execute (tool.rb:81-85). save_image is what calls magick.call, and that's when :inherit_fds reaches execute.

In "reads a source given as a descriptor named in inherit_fds" we exercise that path: the source is a path, so load_image calls convert_shim without a block, and the TIFF is read as /dev/fd/N. (The convert_shim with a block earlier in that test only builds the fixture.) Drop the option inside convert_shim and the test fails with magick: unable to open image '/dev/fd/9': No such file or directory, since the descriptor is close-on-exec and /dev/fd/N isn't there in the child unless spawn's redirect map names it.

Does that answer your question? I'm pretty tired right now, so please tell me if I misunderstood and I'm happy to try to explain more clearly.

[janko]

Makes sense, thanks for the explanation. And how will this be usable from Active Storage? You don't have access to the raw file at the time of the #variant call.

[janko]

Nevermind, Active Storage transformers have access to the raw file, so you could do the plumbing there. Either way it makes sense, as HotCell can be used directly with ImageProcessing gem, from what I understand.