[Python-Dev] Signals, threads, blocking C functions

[Gustavo Carneiro]

On 9/12/06, Adam Olsen <rhamph at gmail.com> wrote:
> On 9/12/06, Gustavo Carneiro <gjcarneiro at gmail.com> wrote:
> > On 9/12/06, Adam Olsen <rhamph at gmail.com> wrote:
> > > 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
>
> Nope.  It'll write 32 bits, then break that up into 8 bits :)
> Although, at the moment I can't fathom what harm that would cause...

  Hmm... it means that to write those 8 bits the processor / compiler
may need to 1. read 32 bits from memory to a register, 2. modify 8
bits of the register, 3. write back those 32 bits.  Shouldn't affect
our case, but maybe it's better to use sig_atomic_t in any case.

> For the record, all volatile does is prevent compiler reordering
> across sequence points.

  It makes the compiler aware the value may change any time, outside
the current context/function, so it doesn't assume a constant value
and always re-reads it from memory instead of assuming a value from a
register is correct.

> > 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);
> >   }
> > }
>
> I'd prefer this to be a "while (signal_flag)" instead, although it
> should technically work either way.

  I guess we can use while instead of if.

>
> > static void
> > signal_handler(int signum)
> > {
> >    char signum_c = signum;
> >    signal_flag = 1;
> >    write(signal_pipe_w, &signum_c, 1);
> > }
>
> This is wrong.  PyErr_CheckSignals could check and clear signal_flag
> before you reach the write() call.  "signal_flag = 1" should come
> after.

  Yes, good catch.

  I don't understand the memory barrier concern in your other email.
I know little on the subject, but from what I could find out memory
barriers are used to avoid reordering of multiple read and write
operations.  However, in this case we have a single value at stake,
there's nothing to reorder.

  Except perhaps that "signal_flag = 0" could be delayed... If it is
delayed until after the while (read (...)...) loop below we could get
in trouble. I see your point now... :|

  But I think that a system call has to act as memory barrier,
forcefully, because the CPU has to jump into kernelspace, a completely
different context, it _has_ to flush pending memory operations sooner
or later.

Round two:

static volatile sig_atomic_t signal_flag;
static int signal_pipe_r, signal_pipe_w;

PyErr_CheckSignals()
{
   while (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;
    write(signal_pipe_w, &signum_c, 1);
    signal_flag = 1;
}

-- 
Gustavo J. A. M. Carneiro
"The universe is always one step beyond logic."