At 08:09 04/04/03 -0800, Alexandre Julliard wrote:
Adam Gundy arg@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.