On Thu, Jun 14, 2001 at 05:10:58PM -0400, Tom Lane wrote:
> Doug McNaught <doug@wireboard.com> writes:
> > Tom Lane <tgl@sss.pgh.pa.us> writes:
> >> Hm. That's one way, but is it really any cleaner than our existing
> >> technique? Since you still need to assume you can do a system call
> >> in a signal handler, it doesn't seem like a real gain in
> >> bulletproofness to me.
>
> > Doing write() in a signal handler is safe; doing fprintf() (and
> > friends) is not.
>
> If we were calling the signal handlers from random places, then I'd
> agree. But we're not: we use sigblock to ensure that signals are only
> serviced at the place in the postmaster main loop where select() is
> called. So there's no actual risk of reentrant use of non-reentrant
> library functions.
>
> Please recall that in practice the postmaster is extremely reliable.
> The single bug we have seen with the signal handlers in recent releases
> was the problem that they were clobbering errno, which was easily fixed
> by saving/restoring errno. This same bug would have arisen (though at
> such low probability we'd likely never have solved it) in a signal
> handler that only invoked write(). So I find it difficult to buy the
> argument that there's any net gain in robustness to be had here.
>
> In short: this code isn't broken, and so I'm not convinced we should
> "fix" it.
Formally speaking, it *is* broken: we depend on semantics that are
documented as unportable and undefined. In a sense, we have been so
unlucky as not to have perceived, thus far, the undefined effects.
This is no different from depending on finding a NUL at *(char*)0, or
on being able to say "free(p); p = p->next;". Yes, it appears to work,
at the moment, on some platforms, but that doesn't make it correct.
It may not be terribly urgent to fix it right now, but that's far from
"isn't broken". It at least merits a TODO entry.
Nathan Myers
ncm@zembu.com