Tatsuo Ishii <t-ishii@sra.co.jp> writes:
>> Seems to me that the correct fix is to have reaper() save and restore
>> the outer value of errno, not to hack the main line to ignore the
>> most probable state left over from reaper(). Otherwise you still choke
>> if some other value gets returned from whatever call reaper() does
>> last.
> Not sure. reaper() may be called while reaper() is executing if a new
> SIGCHLD is raised. How do you handle this case?
No, because the signal is disabled when the trap is taken, and then not
re-enabled until reaper() does pqsignal() just before exiting. We don't
really care if a new signal recursively interrupts reaper() at that
point, but bad things would happen if there were a recursive interrupt
earlier while we were diddling the list of children. (Cf. comments in
the existing code where SIGCHLD is disabled while we add a child...)
In any case, it's not a problem: if each level of reaper does
reaper(){ int save_errno = errno;
...
errno = save_errno; /* restore errno of interrupted code */}
then a recursive interrupt might be saving/restoring the errno value
that existed in the next outer interrupt, rather than the value that
is truly at the outer level, but that's what stacks are for ;-)
>> Moreover, you're not actually checking what the select() did unless
>> you do it that way.
> Sorry, I don't understand this. Can you explain, please?
If you don't have the signal routine save/restore errno, then (when this
problem occurs) you are not seeing the errno returned by the select(),
but one left over from reaper()'s activity. If the select() failed, you
won't know it.
>> Curious that this sort of problem is not seen more often --- I wonder
>> if most Unixes arrange to save/restore errno around a signal handler
>> for you?
> Maybe because the situation I have pointed out is relatively rare.
Well, the window for trouble is awfully tiny in this particular code of
ours, but it might be larger in other programs. Yet I don't think I've
ever heard a programming recommendation to save/restore errno in signal
handlers...
regards, tom lane