Skip to content

client: happy eyeballs: enable POLLOUT on the racing fd, not the primary - #3648

Closed
saghul wants to merge 1 commit into
warmcat:mainfrom
saghul:fix-he-racer-pollout
Closed

client: happy eyeballs: enable POLLOUT on the racing fd, not the primary#3648
saghul wants to merge 1 commit into
warmcat:mainfrom
saghul:fix-he-racer-pollout

Conversation

@saghul

@saghul saghul commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Found while implementing the new parallel-connect event lib ops (sock_accept_parallel / io_parallel / close_handle_manually_parallel / promote_parallel, added in d31f2d830 "event-loop: HE races") in a custom event lib that drives lws off a libuv loop.

On POSIX, lws_client_connect_3_connect() restores the parallel swap — wsi->desc and wsi->position_in_fds_table back to the primary socket — before it does the "hear about the connect completion as a POLLOUT event" lws_change_pollfd():

		if (is_parallel) {
			/* restore swap */
			...
			wsi->desc = saved_fd;
		}
		... suls ...
#else
		if (lws_change_pollfd(wsi, 0, LWS_POLLOUT))
			goto try_next_dns_result_fds;
#endif

So for a racing connect the POLLOUT is applied to the primary socket rather than to the fd connect() was just called on. A racer ends up with only the POLLIN that __insert_wsi_socket_into_fds() seeded, which means it can be seen to have failed (POLLERR/POLLHUP surface on a POLLIN watch) but never to have succeeded:

  • happy eyeballs can never promote a winner on POSIX;
  • if the primary attempt then fails, the racers have already consumed the remaining DNS results, so try_next_dns_result has nothing left and the connection dies with no fallback — which regresses the classic broken-IPv6 case.

That racers are expected to carry POLLOUT is visible in the same function: the quic-race branch explicitly clears it on a connected racer via the swap dance (lws_change_pollfd(wsi, LWS_POLLOUT, 0)).

The fix moves the POLLOUT enable above the swap restore, so it lands on the socket the connect() was issued on in both the primary and the racing case. The goto try_next_dns_result_fds error path already expects the swap to still be active (it restores it itself), so jumping to it from there is unchanged.

Tested on macOS with a libuv-based event lib implementing the parallel ops, with the happy-eyeballs delay temporarily forced to 1us so that every multi-address connect races. Before: https fetches fail with Timed out waiting SSL, or hang on a racer that can never be promoted. With this plus the companion promote fix: all complete normally, the process exits cleanly, full test suite (329 tests) green, ASAN clean.

On POSIX, lws_client_connect_3_connect() restores the parallel swap (wsi->desc
and wsi->position_in_fds_table back to the primary socket) before doing the
"hear about the connect completion as a POLLOUT event" lws_change_pollfd(), so
for a racing (parallel) connect the POLLOUT is applied to the primary socket
instead of the fd that connect() was just called on.

A racer therefore only ever has the POLLIN that __insert_wsi_socket_into_fds()
seeded: it can be seen to have failed (POLLERR/POLLHUP surface on a POLLIN
watch) but never to have succeeded, so happy eyeballs can never promote a
winner, and if the primary attempt fails after racers consumed the remaining
DNS results the connection dies with no fallback.  The quic-race branch in the
same function clears POLLOUT on a connected racer, so racers are clearly
expected to have it.

Move the POLLOUT enable above the swap restore, so it lands on the socket the
connect() was issued on in both the primary and the racing case.
@sonarqubecloud

Copy link
Copy Markdown

@saghul

saghul commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Related happy-eyeballs / parallel-connect fixes from the same investigation, all independent of each other: #3648 (racing fd gets the POLLOUT), #3649 (primary retired when a racer wins), #3650 (fds-removal io stop routed through the parallel ops).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant