Re: Race condition during Dispatcher deconstruction

[Chris Vine <chris cvine freeserve co uk>]

On Tue, 2008-01-08 at 20:45 +0100, Daniel Elstner wrote:
> If I recall correctly we already had an exchange on the list about this
> very topic a while ago. I'll have to look it up, but from memory I think
> I didn't want to move to your alternative implementation because some of
> the design goals of Glib::Dispatcher would have had to be sacrificed; in
> particular the avoidance of mutex locking during normal operation.

I think we did and I think your conclusion was the same; it's a matter
of trade-offs.  Adding a comment to the documentation that the user is
responsible for ensuring a particular Glib::Dispatcher object remains
valid until the last signal emitted on it has been handled, as I think
you propose, would be one way of dealing with it.  (In case I
misunderstood your proposal, it is not a matter of keeping at least one
Glib::Dispatcher alive, it is a matter of keeping the particular
Glib::Dispatcher object which has been invoked alive until its event has
been handled by the receiving thread to prevent emitting on a
non-existent signal object in
Glib::DispatchNotifier::pipe_io_handler().)

Incidentally I noticed, when looking at dispatcher.cc, one thing in the
Glib::DispatchNotifier::pipe_io_handler() method which does not look
quite right.  It is this, when reading from the pipe:

  gssize n_read;

  do
    n_read = read(fd_receiver_, &data, sizeof(data));
  while(G_UNLIKELY(n_read < 0) && errno == EINTR);

This assumes that if a (unix) signal arises, there are only two possible
outcomes: (i) a complete read, or (ii) no read at all with read()
returning -1 and errno set to EINTR.  In fact read() does not have the
same atomic guarantees with a pipe as does write() (and it doesn't need
to, because a pipe is a FIFO so it is atomic writes rather than atomic
reads which are important). Posix enables a signal to cause a part read
- this is from the SUS:

  "... The value returned may be less than nbyte if the number of
  bytes left in the file is less than nbyte, if the read() request was
  interrupted by a signal, or ...

  If a read() is interrupted by a signal before it reads any data, it
  shall return -1 with errno set to [EINTR].

  If a read() is interrupted by a signal after it has successfully read
  some data, it shall return the number of bytes read."

The normal way of dealing with this would be something like this:

  gssize n_read, n_left = sizeof(data);
  char* pos = reinterpret_cast<char*>(&data);
  do
  {
    n_read = read(fd_receiver_, pos, n_left);
    if (n_read > 0)
    {
      pos += n_read;
      n_left -= n_read;
    }
  } while (n_left                               // more to come
           && n_read                            // pipe not closed
           && (n_read != -1 || errno == EINTR); // no error


Chris