Fix wxWebRequest hanging forever with the curl backend (PR #27040)
Gunter Königsman
SourceSocketPoller::StopPolling() defers cleaning up a socket while we are
inside OnWriteWaiting() for it, so as not to delete a handler that a
following error notification would still use (4e0fca8, #24885). The
deferred entry is keyed only on the socket, but curl normally asks us to
poll that same socket again -- for reading, now that the request has been
sent -- before the deferred cleanup runs.
When GLib reports G_IO_OUT and G_IO_ERR in a single notification,
wx_on_channel_event() calls OnWriteWaiting() and then OnExceptionWaiting(),
and the latter drains m_socketsToCleanUp. The entry left by the former then
removes the new read source rather than the obsolete write one, leaving
the socket unwatched: the response sits unread in the kernel receive buffer
and the transfer neither completes nor fails, so the request hangs forever.
Discard any pending deferred cleanup for a socket in StartPolling(): once
curl has asked to poll it again, the source that cleanup referred to has
already been replaced. The connection-refused case fixed by 4e0fca8 is
unaffected, since curl does not restart polling for that socket there.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/27040
Commit Summary
- 48b3324 Don't let a deferred socket cleanup remove the new watch in wxWebRequestCURL
File Changes
(1 file)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/27040.patch
- https://github.com/wxWidgets/wxWidgets/pull/27040.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Gunter Königsman
gunterkoenigsmann left a comment (wxWidgets/wxWidgets#27040)repro.cpp
wx-socketpoller-fix.patch
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#27040)Thanks, the problem is real. I'm not sure this is the best fix for it, however. Let me check if we can't do something better here.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#27040)... I think we can. Instead of adding even more hacks to wxWebRequestCURL we can fix the actual bug in wxGTK event loop, see #27041, and then not only this hack but my original hack from 4e0fca8 (Fix crash when connection is refused in wxWebRequestCURL, 2024-10-27) to fix #24885, which is also fixed by the fix to the event loop.
I'm closing this one, but please test my PR if you can and tell me if you see any problems with it. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.