Thread: Re: dup(0) fails on Ubuntu 20.04 and macOS 10.15 with 13.0

[ redirecting to -hackers ]

Mario Emmenlauer <mario@emmenlauer.de> writes:
> On 05.10.20 14:35, Tom Lane wrote:
>> Mario Emmenlauer <mario@emmenlauer.de> writes:
>>> I get reproducibly the error:
>>> 2020-10-05 11:48:19.720 CEST [84731] WARNING:  dup(0) failed after 0 successes: Bad file descriptor

>> Hmph.  That code loop assumes that stdin exists to be duplicated,
>> but maybe if it had been closed, you'd get this error.
>>
>> However, that logic hasn't changed in decades, and we've not heard
>> complaints about it before.  Are you starting the server in some
>> unusual way?

> Replying to a very old thread here: I could indeed trace the problem of
> the failing `dup(0)` back to how we start the server! We start the
> server from an executable that closes stdin very early on, and this
> seems to lead to the problem.
> We solved it by not closing stdin. But for future reference, if other
> people may be affected by this, the code could probably be modified to
> revert to stdout or stderr or a temporary file in case stdin is not
> available (guessing here...).

Hm.  I'm tempted to propose that we simply change that from dup(0) to
dup(2).  Formally, that's just moving the problem around.  In practice
though, there are good reasons not to close the server's stderr, ie you
will lose error messages that might be important.  OTOH there does not
seem to be any obvious reason why the server should need valid stdin,
so if we can get rid of an implementation artifact that makes it require
that, that seems like an improvement.

            regards, tom lane

On 01.09.21 15:54, Tom Lane wrote:
> Mario Emmenlauer <mario@emmenlauer.de> writes:
>> On 05.10.20 14:35, Tom Lane wrote:
>>> Mario Emmenlauer <mario@emmenlauer.de> writes:
>>>> I get reproducibly the error:
>>>> 2020-10-05 11:48:19.720 CEST [84731] WARNING:  dup(0) failed after 0 successes: Bad file descriptor
> 
>>> Hmph.  That code loop assumes that stdin exists to be duplicated,
>>> but maybe if it had been closed, you'd get this error.
> 
>> Replying to a very old thread here: I could indeed trace the problem of
>> the failing `dup(0)` back to how we start the server! We start the
>> server from an executable that closes stdin very early on, and this
>> seems to lead to the problem.
> 
> Hm.  I'm tempted to propose that we simply change that from dup(0) to
> dup(2).  Formally, that's just moving the problem around.  In practice
> though, there are good reasons not to close the server's stderr, ie you
> will lose error messages that might be important.  OTOH there does not
> seem to be any obvious reason why the server should need valid stdin,
> so if we can get rid of an implementation artifact that makes it require
> that, that seems like an improvement.

The idea to switch to dup(2) sounds very good to me. Also, while at it,
maybe the error message could be improved? The kids nowadays don't learn
so much about system I/O any more, and if someone does not know `dup()`,
then the error message is not very telling. It took me a while to under-
stand what the code was supposed to do. So it may be helpful to add to
the error message something like "possible the stderr stream is closed,
this is not supported". What do you think?

All the best,

    Mario Emmenlauer

Mario Emmenlauer <mario@emmenlauer.de> writes:
> The idea to switch to dup(2) sounds very good to me. Also, while at it,
> maybe the error message could be improved? The kids nowadays don't learn
> so much about system I/O any more, and if someone does not know `dup()`,
> then the error message is not very telling. It took me a while to under-
> stand what the code was supposed to do. So it may be helpful to add to
> the error message something like "possible the stderr stream is closed,
> this is not supported". What do you think?

Meh ... it's been like that for ~20 years and you're the first one
to complain, so I'm not inclined to make our translators spend effort
on a HINT message.  However, we could reword it to the extent of, say,

    elog(WARNING, "duplicating stderr failed after %d successes: %m", used);

which at least reduces the jargon level to something that Unix users
should have heard of.

            regards, tom lane

Mario Emmenlauer <mario@emmenlauer.de> writes:
> The idea to switch to dup(2) sounds very good to me.

I poked at this some more, and verified that adding "fclose(stdin);"
at the head of PostmasterMain is enough to trigger the reported
failure.  However, after changing fd.c to dup stderr not stdin,
we can pass check-world even with that in place.  So that confirms
that there is no very good reason for the postmaster to require
stdin to be available.

Hence, I pushed the fix to make fd.c use stderr here.  I only
back-patched to v14, because given the lack of other complaints,
I couldn't quite justify touching stable branches for this.

            regards, tom lane

On 03.09.21 01:00, Tom Lane wrote:
> Mario Emmenlauer <mario@emmenlauer.de> writes:
>> The idea to switch to dup(2) sounds very good to me.
> 
> I poked at this some more, and verified that adding "fclose(stdin);"
> at the head of PostmasterMain is enough to trigger the reported
> failure.  However, after changing fd.c to dup stderr not stdin,
> we can pass check-world even with that in place.  So that confirms
> that there is no very good reason for the postmaster to require
> stdin to be available.
> 
> Hence, I pushed the fix to make fd.c use stderr here.  I only
> back-patched to v14, because given the lack of other complaints,
> I couldn't quite justify touching stable branches for this.

Thanks a lot Tom, its appreciated!

All the best,

    Mario Emmenlauer