[PATCH 0/1] MR3557: server: Fix 100% CPU usage when a process is suspended (debugged) in WSAPoll

[Torge Matthies (＠tmatthies)]

This has been annoying me for at least 3 years now. I have no idea why the later check for `async_queued( &sock->read_q )` and `async_waiting( &sock->read_q )` doesn't catch this case, but this commit fixes it so idc too much.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3557

[PATCH 1/1] server: Don't consider non-pending poll requests when reselecting socket.

[Torge Matthies]

From: Torge Matthies <openglfreak(a)googlemail.com>

Signed-off-by: Torge Matthies <openglfreak(a)googlemail.com>
---
 server/sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/server/sock.c b/server/sock.c
index ce95b7f2998..7e8192f82a6 100644
--- a/server/sock.c
+++ b/server/sock.c
@@ -1470,6 +1470,8 @@ static int sock_get_poll_events( struct fd *fd )
     {
         unsigned int i;
 
+        if (req->iosb->status != STATUS_PENDING) continue;
+
         for (i = 0; i < req->count; ++i)
         {
             if (req->sockets[i].sock != sock) continue;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3557

[Torge Matthies (＠tmatthies)]

Here's a test case: [poll_test.c](/uploads/ccf69b7248a07ab4de43b17a6612cebd/poll_test.c)

Run the exe with Wine, run `nc 127.0.0.1 12345`, attach to the Wine process with gdb, press Enter in the terminal running the `nc` command. You should see wineserver start to use 100% of a CPU core.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3557#note_42065

[Zebediah Figura (＠zfigura)]

> This has been annoying me for at least 3 years now. I have no idea why the later check for `async_queued( &sock->read_q )` and `async_waiting( &sock->read_q )` doesn't catch this case, but this commit fixes it so idc too much.

Well, in general, one should understand the code, and know whether and why their patches are correct, before sending them...

Anyway, in this case the answer is simple: they're two different mechanisms. Poll asyncs and read asyncs are two different kinds of asyncs, corresponding to WSAPoll() and recv() respectively. As the comment describes (perhaps in a confusing manner), we want to resolve a recv() before signalling a poll, which is why we don't poll for POLLIN until an alerted recv is done being processed. But if there's no recv(), then that logic is irrelevant.

What it sounds like you're hitting is the case where the server has completed a poll and is trying to notify the client of that, so the async has been alerted, but the client, due to its suspension, has not yet received and processed the APC. And this patch is indeed the correct fix for that case.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3557#note_42115

[PATCH 0/1] MR3557: server: Fix 100% CPU usage when a process is suspended (debugged) in WSAPoll - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3557