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

[Joshua Gargus]

John M McIntosh wrote:
>
> Well there is no generic synchronizing code, so you are correct it has
> to be platform specific.
> When I wrote the original code a decade? back in the macintosh os-9
> era there was no agreement
> about doing platform specific implementations since at the time there
> was a tendency to keep all
> the code as generic as possible.
>
> So do you have a test case 

No test case.

> or scenario where the code fails?

Just the obvious race condition.  Let's just look at the first case of
the if-statement; the case for the other buffer is symmetric.  Here's
the code in interp.c

    if (foo->semaphoresUseBufferA) {
        if (foo->semaphoresToSignalCountA < SemaphoresToSignalSize) {
            foo->semaphoresToSignalCountA += 1;
            foo->semaphoresToSignalA[foo->semaphoresToSignalCountA] = index;
        }
    }


Let's say that there are two native threads A and B that want to signal
semaphores with indices 7 and 8, and that there are no other semaphores
to be signalled.  Let's say that thread A is running until just after
"semaphoresToSignalCountA" is incremented, but is interrupted before it
assigns the index.  Then thread B runs, increments
"semaphoresToSignalCountA" again (so its value is now 2), and sets
foo->semaphoresToSignalA[2] = 8.  Then A resumes and stomps this value
by setting foo->semaphoresToSignalA[2] = 7.  Now the semaphore with
index 8 will not be signalled, and just as bad, the index stored in
oo->semaphoresToSignalA[1] is now garbage that will be treated as a
semaphore-index to signal.

Cheers,
Josh



>
> On 2009-09-20, at 12:43 PM, Joshua Gargus 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.
>>
>> Thanks,
>> Josh
>
> -- 
> ===========================================================================
>
> John M. McIntosh <johnmci at smalltalkconsulting.com>   Twitter: 
> squeaker68882
> Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
> ===========================================================================
>
>
>
>
>