client: happy eyeballs: retire the primary socket when a racer wins - #3649
Closed
saghul wants to merge 1 commit into
Closed
client: happy eyeballs: retire the primary socket when a racer wins#3649saghul wants to merge 1 commit into
saghul wants to merge 1 commit into
Conversation
When a racing (parallel) connect is seen to have completed in the
LCCCR_CONNECTED path, promote_parallel_fd() moves wsi->desc and
wsi->position_in_fds_table to the racer, but nothing removes the primary
socket from the fds table or closes it. Its slot keeps mapping the primary fd
to this wsi while the wsi's position_in_fds_table now points at the racer's
slot, so the slot can never be removed: __remove_wsi_socket_from_fds() on this
wsi will always target the racer's slot instead.
The observable effect is that pt->fds never drains, so the
while (pt->fds_count) {
struct lws *wsi = wsi_from_fd(context, pt->fds[0].fd);
...
}
loop in lws_context_destroy() spins forever at 100% CPU, and the primary
socket is leaked.
Retire the primary before promoting, the same way the conn_good is_parallel
path already does (remove from fds, then promote_parallel or close). Reached
easily with an event lib that implements the parallel ops: on Windows the
win32 connect-check sul probes racer fds directly, and on POSIX once the
racing fd actually gets its POLLOUT.
Contributor
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Found while implementing the new parallel-connect event lib ops (
d31f2d830"event-loop: HE races") in a custom event lib driving lws off a libuv loop.When a racing (parallel) connect is seen to have completed, the
LCCCR_CONNECTEDpath inlws_client_connect_3_connect()promotes it withpromote_parallel_fd()alone:Nothing removes the primary socket from the fds table or closes it.
promote_parallel_fd()then moveswsi->position_in_fds_tableto the racer's slot, so the primary's slot is orphaned: it still maps the primary fd to this wsi, but any later__remove_wsi_socket_from_fds()on the wsi targets the racer's slot instead, so that entry can never be removed.Observable effects:
pt->fdsnever drains, so theloop in
lws_context_destroy()spins forever at 100% CPU (the wsi it finds is closed over and over, and its position never matches slot 0). Caught withsampleas an endlesslws_context_destroy→__lws_close_free_wsicycle.the primary socket is leaked.
The
conn_goodis_parallelpath already does this correctly (remove from fds, thenpromote_parallelop orcompatible_close, thenpromote_parallel_fd); this change makes theLCCCR_CONNECTEDpath do the same.__remove_wsi_socket_from_fds()already fixes upparallel_conns[].position_in_fds_tablewhen it moves the end entry into the hole, so the remaining racers' slots stay correct.How easily it is reached, once an event lib implements the parallel ops: on Windows the
win32_sul_connect_async_checksul probes racer fds directly, so any racer that connects before a slow primary lands here; on POSIX it needs the companion fix that gives the racing fd its POLLOUT.Tested on macOS with a libuv-based event lib implementing the parallel ops and the happy-eyeballs delay temporarily forced to 1us, so every multi-address connect races and racers routinely win: before, the process spins at exit after the fetch completes; with this plus the POLLOUT fix, four https fetches complete and the process exits promptly. Full test suite (329 tests) green, ASAN clean.