Keeping the file open allows better prefetching/caching on both
the application layer (ifstream) and OS level (vfs layer) as
the access pattern is sequential and predictable. We also save
three syscalls (open, seek, close) for every chunk we send to
the client (256kb).
Keeping the file open allows better prefetching/caching on both
the application layer (ifstream) and OS level (vfs layer) as
the access pattern is sequential and predictable. We also save
three syscalls (open, seek, close) for every chunk we send to
the client (256kb).
- Don't set FD_CLOEXEC, but close manually before fork:
Since we don't use pipe2() to set the flag atomically, we're prone to
races with concurrent fork+execs either way. As ChildProcess uses a
mutex here anyways, as long as there is no other part in lms that
would fork+exec, we're not any more or less safe now, but the code
is shorter and we cannot fail the fcntl.
- Don't make the write-end of the pipe non-blocking. Usually programs
do not expect this, i.e. they either never check the return code of
a write to stdout (potential data loss), or if they do, they just
bail out on any error, and don't handle EAGAIN. ffmpeg seemed
to handle this fine though (or we were just lucky and always read
the data faster than ffmpeg could produce it).
- Do not close stdin and stderr. Again ffmpeg seems to handle this,
at least regarding stdin thanks to -nostdin, but in general if a
process tries to write to stderr and fd 2 is not open, it might
just bail out. Even worse, the process might have opened some
file it wants to work with, and that file got assigned fd 2 (as
that was the next free fd) - the program would corrupt whatever
file it opened there whenever it tries to write to stderr.
Try to open /dev/null instead for stdin and stderr, and if that
fails, keep whatever lms inherited open, which should be safer
than relying on the child process to handle this case properly.
- exec() does not return on success, no need to check return code,
any return from it is a failure.
- Wrong error message in error path when assigning fd to boost stream.
- F_SETPIPE_SZ requires and int, not size_t. This worked on little
endian systems since the value passed was < 2^32, but on big
endian systems you'd effectively pass "0" if using a 64bit type.
- Address a few clang-tidy complaints (constness)
Simon Rettberg