socket calls in signal handler (WAS: APC + socket restrictions un der Win32?)

On Mon, Mar 08, 2004 at 09:12:52PM +1100, Claudio Natoli wrote:

Hi all,

Was just discussing the issues related to the above off list with Magnus:
http://archives.postgresql.org/pgsql-hackers-win32/2004-03/msg00041.php

Whilst we can think of a number of work-arounds (the simplest being a
suggestion by Magnus: set a flag, like APCcalled, to false before the
select() call, and repeat the select() if APCcalled == true on return from
select), we were wondering if having socket calls inside signal handlers was
a good idea in any case?

Is this a win32 only thing, or is it more general?

OpenBSD for instance has some documentation on which functions
are safe to be called from a signal handler, and socket
operations aren't part of it. See for isntance their manpages
about signal and sigaction.

Kurt

Hi all,

Was just discussing the issues related to the above off list

with Magnus:

http://archives.postgresql.org/pgsql-hackers->win32/2004-03/msg00041.ph

p

Whilst we can think of a number of work-arounds (the simplest being a
suggestion by Magnus: set a flag, like APCcalled, to false before the
select() call, and repeat the select() if APCcalled == true

on return from

select), we were wondering if having socket calls inside

signal handlers was

a good idea in any case?

Is this a win32 only thing, or is it more general?

OpenBSD for instance has some documentation on which functions
are safe to be called from a signal handler, and socket
operations aren't part of it. See for isntance their manpages
about signal and sigaction.

This is very interesting - it points towards this being a general
problem and not just Win32. I was expecting something along this line,
since you should generally be very restrictive with what is done in a
signal handler, and socket I/O can be a lot (say it blocks, etc).

To me this sounds like we have to make a general solution, and not win32
specific, to get the socket calls out of the signal handler. This seems
to be the only one that uses it, so it should be a fairly small change.

Since in this case, it's a simple pid being sent up, it could probably
be done as simple as: (ok, not patch, just text, but..)

1) Change CleanupProc() in postmaster.c to put the pid in a list or an
array instead of calling pgstat_beterm().
2) At the select() call in ServerLoop(), poll this queue and post out
anything that has happened since last loop using pgstat_beterm().

Does this sound reasonable?

//Magnus

"Magnus Hagander" <mha@sollentuna.net> writes:

To me this sounds like we have to make a general solution, and not win32
specific, to get the socket calls out of the signal handler.

Hold on one second here. I thought this thread was discussing some
local problem in the Win32 workaround for lack of signals?

The postmaster's use of nominally unsafe stuff in signal handlers is not
and never has been a problem, because there is only one place in the
main loop where signals are unblocked, thus no possibility for something
to interrupt something else. I don't like the idea of redesigning that
code just because someone misunderstands it.

regards, tom lane

To me this sounds like we have to make a general solution,

and not win32

specific, to get the socket calls out of the signal handler.

Hold on one second here. I thought this thread was discussing some
local problem in the Win32 workaround for lack of signals?

It was from the beginning (not directly the lack of signals, but
misbehaviour of select() with respect to socket functions called on APCs
which are used for signals). But then Kurt pointed out that what we are
doing now may be wrong from other aspects.

What we do now is supposedly unsafe at least on OpenBSD, according to
their manpages:
http://www.openbsd.org/cgi-bin/man.cgi?query=signal&amp;apropos=0&amp;sektion=0&amp;
manpath=OpenBSD+Current&arch=i386&format=html

It says:
"Most functions not in the above lists are considered to be unsafe with
respect to signals. That is to say, the behaviour of such
functions when
called from a signal handler is undefined."

This sounds a bit scary to me. (There are no socket functions on the
list, so the pgstat_beterm behaviour is undefined at least on OpenBSD
from what I can tell)

The postmaster's use of nominally unsafe stuff in signal
handlers is not
and never has been a problem, because there is only one place in the
main loop where signals are unblocked, thus no possibility for
something
to interrupt something else. I don't like the idea of redesigning that
code just because someone misunderstands it.

You're saying the above is not valid because we block signals?

The issue specific to win32 is connected to the select() call, which is
indeed in such an area of the code. But the reference above to OpenBSD
appears to be for signal handlers in general. And that should not be
affected by signal blocking, no?

It clearly works now, but it sounds like a dangerous path to me. But it
can certainly be me misunderstanding the whole thing :-)

A localized win32 fix is probably a bit easier to do (just a wrapper
around select), but I figured you'd want the generic case taken care of.
Just let me/us know which is preferred.

//Magnus

On Mon, Mar 08, 2004 at 11:33:17PM +0100, Magnus Hagander wrote:

The postmaster's use of nominally unsafe stuff in signal
handlers is not
and never has been a problem, because there is only one place in the
main loop where signals are unblocked, thus no possibility for
something
to interrupt something else. I don't like the idea of redesigning that
code just because someone misunderstands it.

You're saying the above is not valid because we block signals?

It's all about reentrance of functions where it's not safe to do
so.

Either you avoid it in the signal handler or you avoid it by only
allowing it during a "safe" period.

Kurt

The postmaster's use of nominally unsafe stuff in signal
handlers is not
and never has been a problem, because there is only one place in the
main loop where signals are unblocked, thus no possibility for
something
to interrupt something else. I don't like the idea of

redesigning that

code just because someone misunderstands it.

You're saying the above is not valid because we block signals?

It's all about reentrance of functions where it's not safe to do
so.

Either you avoid it in the signal handler or you avoid it by only
allowing it during a "safe" period.

Ok. Let me try to get this straight :-)

Since we *only* permit signals during the select() call, we are safe as
long as we don't call select() inside the signal handlers? (since
select() is not on the list of safe functions). Which we don't.

That makes sense :-)

If that is indeed the case, I withdraw all my comments and misdirected
ideas, and say we go for a win32 specific workaround :-)

//Magnus