Re: [Libguestfs] [PATCH nbdkit] sh: Remove assert and replace with smarter file descriptor duplication. (was: Re: [nbdkit PATCH v2 14/17] sh: Use pipe2 with CLOEXEC when possible)

On 8/27/19 6:47 AM, Richard W.M. Jones wrote: > On Fri, Aug 02, 2019 at 02:26:15PM -0500, Eric Blake wrote: >> + /* Ensure that stdin/out/err of the current process were not empty >> + * before we started creating pipes (otherwise, the close and dup2 >> + * calls below become more complex to juggle fds around correctly). >> + */ >> + assert (in_fd[0] > STDERR_FILENO && in_fd[1] > STDERR_FILENO && >> + out_fd[0] > STDERR_FILENO && out_fd[1] > STDERR_FILENO && >> + err_fd[0] > STDERR_FILENO && err_fd[1] > STDERR_FILENO); > > This assert is now being thrown whenever we use nbdkit + sh plugin + > nbdkit -s option. It's easy to demonstrate using a libnbd one-liner: >

Hmm, so we've closed stdin/out because the client connection is no longer around, but still give the shell script one more callback. Yeah, the failure is definitely explainable, and worth fixing. > We can easily paper over this by ignoring the assert on the close > path, but I foresee two problems: > > (1) I don't believe the assert is generally correct. While it's not > ideal for nbdkit to be run with stdin/out/err closed (they obviously > should be connected to /dev/null) it's also not impossible for that to > happen. We don't cope well with this situation.

But it's close; so I'll use it as the starting point and push the corrected version soon.