[Vm-dev] Why isn't signalSemaphoreWithIndex() thread-safe?

[Eliot Miranda]

On Sun, Sep 20, 2009 at 3:02 PM, Joshua Gargus <schwa at fastmail.us> wrote:

>
> Eliot Miranda wrote:
> >
> >
> > ------------------------------------------------------------------------
> >
> >
> >
> > On Sun, Sep 20, 2009 at 1:00 PM, Eliot Miranda
> > <eliot.miranda at gmail.com <mailto:eliot.miranda at gmail.com>> wrote:
> >
> >     Hi Josh,
> >
> >     On Sun, Sep 20, 2009 at 12:43 PM, Joshua Gargus <schwa at fastmail.us
> >     <mailto:schwa at fastmail.us>> wrote:
> >
> >
> >         Hi,
> >
> >         I've always assumed that signalSemaphoreWithIndex() must be
> >         thread-safe;
> >         after all, it's the "official" mechanism for notifying the
> >         image that an
> >         asynchronous event has occurred.  However, a pang of paranoia
> >         prompted
> >         me to actually look at the code, and it seems clearly unsafe.
> >          This is
> >         bad, because I've been using it to signal events from separate
> >         native
> >         threads.
> >
> >         What should we do about this?  It seems to me that it should
> >         be wrapped
> >         in a critical section, using the appropriate platform-specific
> >         synchronization primitives.
> >
> >
> >     There's no need for this heavyweight approach because the VM can
> >     only respond to these signals synchronously.
> >
> >
> > On second reading I think what I wrote below is exactly what you
> > implied.  Sorry.
>
> Nope, you're always giving me too much credit ;-)  I wasn't thinking
> about any particular implementation of thread-safety, only raising the
> issue that (in my understanding) we don't have it.
>
> >
> >
> >
> >      Instead we can use three variables per index to implement an
> >     exclusion-free solution, thusly:
> >
> >     typedef struct { int lock; // accessed via e.g. XCHG to protect
> >     signalRequest
> >                            int requests;
> >                            int responses;
> >                } ExternalSignalRequest;
> >
> >     ExternalSignalRequest *externalSignalRequests;
> >     int checkExternalSignalRequests;
> >
> >     void
> >     requestSignal(int i)
> >     {
> >            while (!lock(&externalSignalRequests[i].lock))
> >                  usleep(1);
> >
> >            ++externalSignalRequests[i].requests;
> >
> >             unlock(&externalSignalRequests[i].lock);
> >
> >             checkExternalSignalRequests = 1;
> >             forceInterruptCheck();  // set a flag to cause the VM to
> >     check for interrupts; in the stack VM this is stackLimit :=
> >     (usqInt)-1;
> >     }
> >
> >
> >     The VM responds in checkEvents:
> >
> >         if (checkExternalSignalRequests)
> >             for (i = 0; i < numExternalSemaphores; i++)
> >                 while (externalSignalRequests[i]. responses
> >     != externalSignalRequests[i]. requests) {
> >                     signalSemaphoreWithIndex(i);
> >                     externalSignalRequests[i]. responses;
> >                 }
> >
>
>
> Am I correct that ExternalSignalRequest.reponses is never reset to zero,
> instead eventually wrapping around?  That worried me for a minute, but I
> see that it doesn't matter.  Neeto.
>

Yes.  This could be Allen Shiffman's or Peter Deutsch's idea.  Its used in
the VW VM.  Very nice.  It can only fail if the VM ignores at least 2^32
successive requests :)


> Your solution looks good; it's certainly nicer than a heavyweight
> critical section, especially since contention will be infrequent.
>

Right.  Of course if any one external semaphore is never signalled from more
than one thread we don't even need that but the cost of the XCHG should be
very low compared to the overall cost of the signal.


> Cheers,
> Josh
>
>
>
>
> >
> >
> >         Thanks,
> >         Josh
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20090920/ad973c2b/attachment.htm