valgrind for WINE

Adam Gundy arg at cyberscience.com
Fri Apr 4 10:57:51 CST 2003


At 08:09 04/04/03 -0800, Alexandre Julliard wrote:
>Adam Gundy <arg at cyberscience.com> writes:
>
>> I can currently produce this race (without my patch applied) by adding a "Sleep( 1000 )"
>> to the USR1 signal handler in ntdll/signal_i386.c. The wineserver is convinced that the
>> thread is suspended, but it isn't (sleeping is a bad example - assume that a context
>> switch occurred, or heavy processing happened before the signal was delivered).
>
>Not sure what you are trying to prove here; if the thread is inside

just that the thread to be suspended may run for some time before it stops. signal
delivery is NOT guaranteed to be immediate on SMP.

it's not guaranteed to be immediate on UP either - harder to trigger, but suppose
other signals are pending for the thread to be suspended; they could be handled first...

since we block SIGUSR1 delivery during client/server interaction , the thread to be
suspended could wait a (potentially) long time before it is suspended.

>the SIGUSR1 handler then it is already suspended, it doesn't matter if
>you sleep or not. What matters is how much code it runs before getting
>the signal.

but it doesn't matter how much code it runs. suspending a thread under Windows is NOT
guaranteed to stop it immediately - just some time fairly soon. To quote Microsoft's
documentation:

  This function [SuspendThread] is primarily designed for use by debuggers.
  It is not intended to be used for thread synchronization.

>> this 'race' doesn't have ANY effect, because suspending a thread is not a precise operation -
>> it doesn't matter whether 1 or 1000 instructions are executed by the
>> thread before it stops.
>
>It certainly matters if the thread never stops, which can easily
>happen with your patch, since anything can happen to the thread that
>was supposed to send the SIGUSR1.

anything? there is a very small chance that _another_ thread will suspend or kill the
responsible thread before it can send the signal. code that does that won't work reliably
anyway, since the second suspend could just have easily have happened before the first.

this is getting confusing. maybe it's time to start drawing ascii art ;-)

can you honestly come up with a case where this change is going to affect anyone??
the only code I can think of that might behave differently is if someone is trying to
do thread synchronization using SuspendThread - which Microsoft explicitly say won't work...

if you are really worried about the above case, we could block SIGUSR1/SIGKILL for the
duration of the suspendthread client/server interaction - we already block these signals
during 'wine_server_call()' - it's just a case of extending this blocking around the other
code in SuspendThread().


Seeya,
 Adam
--
Real Programmers don't comment their code. If it was hard to write,
it should be hard to read, and even harder to modify.
These are all my own opinions.




More information about the wine-devel mailing list