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

Igor Stasenko siguctua at gmail.com
Sun Sep 20 22:23:05 UTC 2009


2009/9/21 Joshua Gargus <schwa at fastmail.us>:
>
> 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.
>

Nope, it does resets the counters.. see signalExternalSemaphores  method.
...
			semaphoresToSignalCountB = 0;
...
			semaphoresToSignalCountA = 0;
...
and btw, this method is non-thread-safe, despite the
synchronizedSignalSemaphoreWithIndex() function.

Consider the code:
*****         semaphoresUseBufferA = !semaphoresUseBufferA;
		xArray = longAt((specialObjectsOop + BaseHeaderSize) +
(ExternalObjectsArray << ShiftForWord));
		xSize = stSizeOf(xArray);
		if (semaphoresUseBufferA) {

if non-VM thread is interrupted in the middle of signaling, just after
it checked the semaphoresUseBufferA flag,
but not yet added the signal to buffer, and OS deside to switch to VM
thread which enters the signalExternalSemaphores method, then signal
will be lost.

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



-- 
Best regards,
Igor Stasenko AKA sig.


More information about the Vm-dev mailing list