Signals, threads, blocking C functions

I was hoping to have stopped, but here are a few comments.

I agree with Jan Kanis.  That is the way to tackle this one.

"Adam Olsen" <rhamphgmail.com> wrote:
>         
> I don't think we should let this die, at least not
yet.  Nick seems to
> be arguing that ANY signal handler is prone to random
crashes or
> corruption (due to bugs).  However, we already have a
signal handler,
> so we should already be exposed to the random
crashes/corruption.

No.  I am afraid that is a common myth and often
catastrophic mistake.
In this sort of area, NEVER assume that even apparently
unrelated changes
won't cause 'working' code to misbehave.  Yes, Python is
already exposed,
but it would be easy to turn a very rare failure into a more
common one.

What I was actually arguing for was defensive programming.

> If we're going to rely on signal handling being
correct then I think
> we should also rely on write() being correct.  Note
that I'm not
> suggesting an API that allows arbitrary signal
handlers, but rather
> one that calls write() on an array of prepared file
descriptors
> (ignoring errors).

For your interpretation of 'correct'.  The cause of this
chaos is that
the C and POSIX standards are inconsistent, even internally,
and they
are wildly incompatible.  So, even if things 'work' today,
don't bet on
the next release of your favourite system behaving the same
way.

It wouldn't matter if there was a de facto standard (i.e. a
consensus),
but there isn't.

> Ensuring modifications to that array are atomic would
be tricky, but I
> think it would be doable if we use a read-copy-update
approach (with
> two alternating signal handler functions).  Not sure
how to ensure
> there's no currently running signal handlers in
another thread though.
>  Maybe have to rip the atomic read/write stuff out of
the Linux
> sources to ensure it's *always* defined behavior.

Yes.  But even that wouldn't solve the problem, as that
code is very
gcc-specific.

> Looking into the existing signalmodule.c, I see no
attempts to ensure
> atomic access to the Handlers data structure.  Is the
current code
> broken, at least on non-x86 platforms?

Well, at a quick glance at the actual handler (the riskiest
bit):

    1) It doesn't check the signal range - bad practice, as
systems
do sometimes generate wayward numbers.

    2) Handlers[sig_num].tripped = 1; is formally undefined,
but
actually pretty safe.  If that breaks, nothing much will
work.  It
would be better to make the int sig_atomic_t, as you say.

    3) is_tripped++; and
Py_AddPendingCall(checksignals_witharg, NULL);
will work only because the handler ignores all signals in
subthreads
(which is definitely NOT right, as the comments say).

Despite the implication, the code of Py_AddPendingCall is
NOT safe
against simultaneous registration.  It is just plain broken,
I am
afraid.  The note starting "Darn" should be a
LOT stronger 

[ For example, think of two threads calling the function at
exactly
the same time, in almost perfect step.  Oops. ]

I can't honestly promise to put any time into this in the
forseeable
future, but will try (sometime).  If anyone wants to tackle
this,
please ask me for comments/help/etc.


Regards,
Nick Maclaren,
University of Cambridge Computing Service,
New Museums Site, Pembroke Street, Cambridge CB2 3QH,
England.
Email:  nmm1cam.ac.uk
Tel.:  +44 1223 334761    Fax:  +44 1223 334679
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

On 9/9/06, Nick Maclaren <nmm1cus.cam.ac.uk> wrote:
> I was hoping to have stopped, but here are a few
comments.
>
> I agree with Jan Kanis.  That is the way to tackle this
one.

  Alas, it doesn't work in practice, as I already replied.

[...]
> Despite the implication, the code of Py_AddPendingCall
is NOT safe
> against simultaneous registration.  It is just plain
broken, I am
> afraid.  The note starting "Darn" should be
a LOT stronger 

  Considering that this code has existed for a very long
time, and
that it isn't really safe, should we even bother to try to
make
signals 100% reliable?

  I remember about a security-related module (bastion?) that
first
claimed to allow execution of malicious code while
protecting the
system; later, they figured out it wasn't really safe, and
couldn't be
safe, so the documentation was simply changed to state not
to use that
module if you need real security.

  I see the same problem here.  Python signal handling
isn't _really_
100% reliable.  And it would be very hard to make
Py_AddPendingCall /
Py_MakePendingCalls completely reliable.

But let's think for a moment.  Do we really _need_ to make
Python unix
signal handling 100% reliable?  What are the uses for
signals?  I can
only understand a couple of uses: handling of SIGINT for
generating
KeyboardInterrupt [1], and handling of fatal errors like
SIGSEGV in
order to show a crash dialog and bug reporting tool.  The
second use
case doesn't demand 100% reliability.  The second use case
is
currently being handled also in recent Ubuntu Linux through
/proc/sys/kernel/crashdump-helper.  Other notable uses that
I see of
signals are sending SIGUSR1 or SIGHUP to a daemon to make it
reload
its configuration.  But any competent programmer already
knows how to
make the program use local sockets instead.


[1] Although ideally Python wouldn't even have
KeyboardInterrupt and
just die on Ctrl-C like any normal program.
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

On 9/9/06, Nick Maclaren <nmm1cus.cam.ac.uk> wrote:
> I can't honestly promise to put any time into this in
the forseeable
> future, but will try (sometime).  If anyone wants to
tackle this,
> please ask me for comments/help/etc.

It took me a while to realize just what was wrong with my
proposal,
but I did, and it led me to a new proposal.  I'd appreciate
if you
could point out any holes in it.  First though, for the
benefit of
those reading, I'll try to explain the (multiple!) reasons
why mine
fails.

First, sig_atomic_t essentially promises that the compiler
will behave
atomically and the CPU it's ran on will behave locally
atomic.  It
does not claim to make writes visible to other CPUs in an
atomic way,
and thus you could have different bytes show up at different
times.
The x86 architecture uses a very simple scheme and won't do
this
(unless the compiler itself does), but other architectures
will.

Second, the start of a write call may be delayed a very long
time.
This means that a fd may not be written to for hours until
after the
signal started.  We can't release any fd's used for such a
purpose, or
else risk random writing to them if they get reused later..

Third, it doesn't resolve the existing problems.  If I'm
going to fix
signals I should fix ALL of signals. 

Now on to my new proposal.  I do still use write().  If you
can't
accept that I think we should rip signals out entirely, just
let them
kill the process.  Not a reliable feature of any OS.

We create a single pipe and use it for all signals.  We
never release
it, instead letting the OS do it when the process gets
cleaned up.  We
write the signal number to it as a byte (assuming there's
at most 256
unique signals).

This much would allow a GUI's poll loop to wake up when
there is a
signal, and give control back to the python main loop, which
could
then read off the signals and queue up their handler
functions.

The only problem is when there is no GUI poll loop.  We
don't want
python to have to poll the fd, we'd rather it just check a
variable.
Is it possible to set/clear a flag in a sufficiently
portable
(reentrant-safe, non-blocking, thread-safe) fashion?

-- 
Adam Olsen, aka Rhamphoryncus
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

On 9/11/06, Adam Olsen <rhamphgmail.com> wrote:
> Now on to my new proposal.  I do still use write().  If
you can't
> accept that I think we should rip signals out entirely,
just let them
> kill the process.  Not a reliable feature of any OS.
>
> We create a single pipe and use it for all signals.  We
never release
> it, instead letting the OS do it when the process gets
cleaned up.  We
> write the signal number to it as a byte (assuming
there's at most 256
> unique signals).
>
> This much would allow a GUI's poll loop to wake up
when there is a
> signal, and give control back to the python main loop,
which could
> then read off the signals and queue up their handler
functions.

  I like this approach.  Not only we would get a poll-able
file
descriptor to notify a GUI main loop when signals arrive,
we'd also
avoid the lack of async safety in Py_AddPendingCall /
Py_MakePendingCalls which affects _current_ Python code.

  Note that the file descriptor of the read end of the pipe
has to
become a public Python API so that 3rd party extensions may
poll it.
This is crucial.

>
> The only problem is when there is no GUI poll loop.  We
don't want
> python to have to poll the fd, we'd rather it just
check a variable.
> Is it possible to set/clear a flag in a sufficiently
portable
> (reentrant-safe, non-blocking, thread-safe) fashion?

  It's simple.  That pipe file descriptor has to be changed
to
non-blocking mode in both ends of the pipe, obviously, with
fcntl.
Then, to find out whether a signal happened or not we modify
PyErr_CheckSignals() to try to read from the pipe.  If it
reads bytes
from the pipe, we process the corresponding python signal
handlers or
raise KeyboardInterrupt.  If the read() syscall returns zero
bytes
read, we know no signal was delivered and move on.

  The only potential problem left is that, by changing the
pipe file
descriptor to non-blocking mode we can only write as many
bytes to it
without reading from the other side as the pipe buffer
allows.  If a
large number of signals arrive very quickly, that buffer may
fill and
we lose signals.  But I think the default buffer should be
more than
enough.  And normally programs don't receive lots of
signals in a
small time window.  If it happens we may lose signals, but
that's very
rare, and who cares anyway.

  Regards.
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

On 9/11/06, Gustavo Carneiro <gjcarneirogmail.com> wrote:
> On 9/11/06, Adam Olsen <rhamphgmail.com> wrote:
> > This much would allow a GUI's poll loop to wake
up when there is a
> > signal, and give control back to the python main
loop, which could
> > then read off the signals and queue up their
handler functions.
>
>   I like this approach.  Not only we would get a
poll-able file
> descriptor to notify a GUI main loop when signals
arrive, we'd also
> avoid the lack of async safety in Py_AddPendingCall /
> Py_MakePendingCalls which affects _current_ Python
code.
>
>   Note that the file descriptor of the read end of the
pipe has to
> become a public Python API so that 3rd party extensions
may poll it.
> This is crucial.

Yeah, so long as Python still does the actual reading.


> > The only problem is when there is no GUI poll
loop.  We don't want
> > python to have to poll the fd, we'd rather it
just check a variable.
> > Is it possible to set/clear a flag in a
sufficiently portable
> > (reentrant-safe, non-blocking, thread-safe)
fashion?
>
>   It's simple.  That pipe file descriptor has to be
changed to
> non-blocking mode in both ends of the pipe, obviously,
with fcntl.
> Then, to find out whether a signal happened or not we
modify
> PyErr_CheckSignals() to try to read from the pipe.  If
it reads bytes
> from the pipe, we process the corresponding python
signal handlers or
> raise KeyboardInterrupt.  If the read() syscall returns
zero bytes
> read, we know no signal was delivered and move on.

Aye, but my point was that a syscall is costly, and we'd
like to avoid
it if possible.

We'll probably have to benchmark it though, to find out if
it's worth
the hassle.


>   The only potential problem left is that, by changing
the pipe file
> descriptor to non-blocking mode we can only write as
many bytes to it
> without reading from the other side as the pipe buffer
allows.  If a
> large number of signals arrive very quickly, that
buffer may fill and
> we lose signals.  But I think the default buffer should
be more than
> enough.  And normally programs don't receive lots of
signals in a
> small time window.  If it happens we may lose signals,
but that's very
> rare, and who cares anyway.

Indeed, we need to document very clearly that:
* Signals may be dropped if there is a burst
* Signals may be delayed for a very long time, and if you
replace a
previous handler your new handler may get signals intended
for the old
handler

-- 
Adam Olsen, aka Rhamphoryncus
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

Gustavo Carneiro wrote:
>   The only potential problem left is that, by changing
the pipe file
> descriptor to non-blocking mode we can only write as
many bytes to it
> without reading from the other side as the pipe buffer
allows.  If a
> large number of signals arrive very quickly, that
buffer may fill and
> we lose signals.

That might be an argument for *not* trying to
communicate the signal number by the value
written to the pipe, but keep a separate set
of signal-pending flags, and just use the pipe
as a way of indicating that *something* has
happened.

-- 
Greg Ewing, Computer Science Dept,
+--------------------------------------+
University of Canterbury,	   | Carpe post meridiem!         
	  |
Christchurch, New Zealand	   | (I'm not a morning person.) 
        |
greg.ewingcanterbury.ac.nz	  
+--------------------------------------+
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

On 9/11/06, Greg Ewing <greg.ewingcanterbury.ac.nz> wrote:
> Gustavo Carneiro wrote:
> >   The only potential problem left is that, by
changing the pipe file
> > descriptor to non-blocking mode we can only write
as many bytes to it
> > without reading from the other side as the pipe
buffer allows.  If a
> > large number of signals arrive very quickly, that
buffer may fill and
> > we lose signals.
>
> That might be an argument for *not* trying to
> communicate the signal number by the value
> written to the pipe, but keep a separate set
> of signal-pending flags, and just use the pipe
> as a way of indicating that *something* has
> happened.

That brings you back to how you access the flags variable. 
At best it
is very difficult, requiring unique assembly code for every
supported
platform.  At worst, some platforms may not have any way to
do it from
an interrupt context..

A possible alternative is to keep a set of flags for every
thread, but
that requires the threads poll their variable regularly, and
possibly
a wake-up pipe for each thread..

-- 
Adam Olsen, aka Rhamphoryncus
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

Adam Olsen wrote:

> That brings you back to how you access the flags
variable.

The existing signal handler sets a flag, doesn't it?
So it couldn't be any more broken than the current
implementation.

If we get too paranoid about this, we'll just end
up deciding that signals can't be used for anything,
at all, ever. That doesn't seem very helpful,
although techically I suppose it would solve
the problem. 

My own conclusion from all this is that if you
can't rely on writing to a variable in one part
of your program and reading it back in another,
then computer architectures have become far
too clever for their own good. :-(

--
Greg
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

On 9/12/06, Greg Ewing <greg.ewingcanterbury.ac.nz> wrote:
> Adam Olsen wrote:
>
> > That brings you back to how you access the flags
variable.
>
> The existing signal handler sets a flag, doesn't it?
> So it couldn't be any more broken than the current
> implementation.
>
> If we get too paranoid about this, we'll just end
> up deciding that signals can't be used for anything,
> at all, ever. That doesn't seem very helpful,
> although techically I suppose it would solve
> the problem. 
>
> My own conclusion from all this is that if you
> can't rely on writing to a variable in one part
> of your program and reading it back in another,
> then computer architectures have become far
> too clever for their own good. :-(

They've been that way for a long, long time.  The irony is
that x86 is
immensely stupid in this regard, and as a result most
programmers
remain unaware of it.

Other architectures have much more interesting read/write
and cache
reordering semantics, and the code is certainly broken
there.  C
leaves it undefined with good reason.

My previous mention of using a *single* flag may survive
corruption
simply because we can tolerate false positives.  Signal
handlers would
write 0xFFFFFFFF, the poll loop would check if *any* bit is
set.  If
so, write 0x0, read off the fd, then loop around and check
it again.
If the start of the read() acts as a write-barrier it SHOULD
guarantee
we don't miss any positive writes.

Hmm, if that works we should be able to generalize it for
all the
other flags too.  Something to think about anyway...

-- 
Adam Olsen, aka Rhamphoryncus
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com

On 9/12/06, Adam Olsen <rhamphgmail.com> wrote:
> On 9/12/06, Greg Ewing <greg.ewingcanterbury.ac.nz> wrote:
> > Adam Olsen wrote:
> >
> > > That brings you back to how you access the
flags variable.
> >
> > The existing signal handler sets a flag, doesn't
it?
> > So it couldn't be any more broken than the
current
> > implementation.
> >
> > If we get too paranoid about this, we'll just end
> > up deciding that signals can't be used for
anything,
> > at all, ever. That doesn't seem very helpful,
> > although techically I suppose it would solve
> > the problem. 
> >
> > My own conclusion from all this is that if you
> > can't rely on writing to a variable in one part
> > of your program and reading it back in another,
> > then computer architectures have become far
> > too clever for their own good. :-(
>
> They've been that way for a long, long time.  The
irony is that x86 is
> immensely stupid in this regard, and as a result most
programmers
> remain unaware of it.
>
> Other architectures have much more interesting
read/write and cache
> reordering semantics, and the code is certainly broken
there.  C
> leaves it undefined with good reason.
>
> My previous mention of using a *single* flag may
survive corruption
> simply because we can tolerate false positives.  Signal
handlers would
> write 0xFFFFFFFF, the poll loop would check if *any*
bit is set.  If
> so, write 0x0, read off the fd, then loop around and
check it again.
> If the start of the read() acts as a write-barrier it
SHOULD guarantee
> we don't miss any positive writes.

  Why write 0xFFFFFFFF?  Why can't the variable be of a
"volatile
char" type?  Assuming sizeof(char) == 1, please don't
tell me
architecture XPTO will write the value 4 bits at a time! :P

  I see your point of using a flag to avoid the read()
syscall most of
the time.  Slightly more complex, but possibly worth it.

  I was going to describe a possible race condition, then
wrote the
code below to help explain it, modified it slightly, and now
I think
the race is gone.  In any case, the code might be helpful to
check if
we are in sync.  Let me know if you spot any  race condition
I missed.


static volatile char signal_flag;
static int signal_pipe_r, signal_pipe_w;

PyErr_CheckSignals()
{
  if (signal_flag) {
     char signum;
     signal_flag = 0;
     while (read(signal_pipe_r, &signum, 1) == 1)
         process_signal(signum);
  }
}

static void
signal_handler(int signum)
{
   char signum_c = signum;
   signal_flag = 1;
   write(signal_pipe_w, &signum_c, 1);
}
_______________________________________________
Python-Dev mailing list
Python-Devpython.org
ht
tp://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/p
ython-dev/nessto%40sharedlog.com