[PATCH 01/15] server: Wait before suspending threads in APC.

[Jacek Caban]

On 27.01.2020 23:19, Rémi Bernon wrote:
> On 1/27/20 10:58 PM, Jacek Caban wrote:
>> Hi Rémi,
>>
>>
>> On 27/01/2020 13:07, Rémi Bernon wrote:
>>> The APC_BREAK_PROCESS call creates a thread that raises an exception,
>>> suspending the whole process. There's a race condition between this
>>> thread and the APC thread notifying the server of its completion.
>>
>>
>> What's wrong about that? Suspending process on thread creation 
>> shouldn't be a problem. The debugger should process this event, which 
>> will resume the new thread and let it continue to the break point. 
>> Before introducing APC_BREAK_PROCESS I had to fix it in another place 
>> in winedbg, see commit 1bb98982d682a85b82e, maybe there is similar 
>> problem in gdb proxy?
>>
>>
>> Thanks,
>>
>> Jacek
>>
>
> In gdbproxy mode, winedbg calls DebugBreakProcess whenever Ctrl-C is 
> pressed, then WaitForDebugEvent for the corresponding event.
>
> However *sometimes* DebugBreakProcess never returns and it cannot wait 
> for the event and process it unless it is done in another thread 
> -which would require changes in gdbproxy I would rather avoid. That 
> may be how winedbg works in frontend mode?
>
> The reason why DebugBreakProcess sometimes returns but not always is 
> that it requests the APC, which completion notification is sometimes 
> received by the server before the exception is raised and all threads 
> are suspended, but sometimes it isn't. I don't think this was 
> intended, and it would probably be better if DebugBreakProcess returns 
> -or not- in a consistent way.


OK, I see, there is a problem. But I think that with your patch the race 
is still there, it's just much less likely. Server may have already sent 
the signal before client sent select request, but it may be blocked 
until server call is finishes. Then, if select returns system APC, it 
will be immediately interrupted by the signal.


Jacek