Skip to content

Bound TCP resolution and connection waits with real deadlines - #34

Merged
284km merged 2 commits into
mainfrom
fix/tcp-connect-deadlines
Nov 7, 2026
Merged

284km merged 2 commits into
mainfrom
fix/tcp-connect-deadlines

Conversation

@284km

@284km 284km commented Nov 7, 2026 •

Copy link
Copy Markdown
Owner

Ruby's TCP test file hangs after rejecting test_mode_settings: its ensure clause waits for an accept thread that never receives a connection. Implement deadline-aware TCP connection establishment, with native resolver workers and nonblocking numeric connect attempts, so all 22 tests complete.

The Ruby scheduling loop handles resolver delay/error controls, IPv6 preference, IPv4 fallback, open/resolution/connection deadlines, and cleanup of losing descriptors and pending resolutions. When resolution expires, already resolved addresses and active attempts remain usable; if all attempts fail, the resolution timeout retains precedence over a connection error. Requested local addresses and ports are bound before connecting. Also preserve nil at TCP EOF and return the correct nil sender for actual recvfrom payloads and expose actual interface addresses via getifaddrs, preserving IPv6 scope IDs. Socket#connect retains its existing implementation; its limitation is documented.

Validation:

  • Ruby 4.0.9 socket/test_tcp.rb: 22 tests, 27 assertions, no failures/errors, about 3 seconds for the complete process (previously a 600-second timeout).
  • Added explicit regressions for TCP EOF and partial resolution before the IPv6 preference wait expires; both match Ruby with the hash index ON and OFF.
  • Added a bounded regression for resolver deadlines, family fallback, cleanup, local binding and interface addresses. It matches on macOS and Linux in both modes.
  • Native checks cover resolver cancellation and completion, descriptor flags, poll/SO_ERROR completion, binding errors, interface-address representation, and resolver use after fork.
  • New checks run in CI; native glue compiles with -Wall -Wextra -Werror.

Native lifecycle checks also passed on Linux. The combined #32–#36 implementation on main f7ee263 passes all 515 corpus programs with hash indexing ON and OFF. New-head CI is in progress.

CI for this head passed all 29 jobs. All 758 CRuby test files were collected from 8 shards with no missing files or infrastructure errors. This confirms complete measurement, not full Ruby conformance. The local integration of #32–#36 with main f7ee263 also passed all 515 corpus programs with hash indexing ON and OFF. The full Unicode suite completed within the unchanged limit with the same 68 baseline failure/error reports. The TCP regressions and native compaction-deferral checks also passed.

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 637a705 against 7ac8c4c. Found two Major issues to address before merging:

  1. TCPSocket#recvfrom now returns [nil, nil] at EOF instead of nil, breaking callers that terminate on a nil result.
  2. The resolution deadline aborts a connection even when another family has already supplied a usable address. With a reachable IPv4 listener and delayed IPv6 resolution, Ruby 4.0.9 connects while this implementation raises IO::TimeoutError.

Both issues reproduce with hash indexing enabled and disabled; details and suggested fixes are inline.

The native worker ownership is a good part of this change: cancellation releases the caller immediately while the worker retains ownership of its libc allocations until it finishes.

Validation:

  • Fresh O2 build using the CI-pinned Mere 991f299b compiler and mgz revision.
  • unittest/socket_glue_check.py passed, including strict compiler warnings, cancellation, fork, nonblocking connection flags, binding errors, and interface-address representation.
  • unittest/tcp_connect_check.py matched Ruby 4.0.9 in both index modes. The new corpus regression also passed locally with MERE_RUBY_GC_STRESS=100.
  • The repository's CRuby test runner reports 22/22 matching TCP tests, 27 assertions, zero failures/errors on both interpreters. The two additional cases above expose gaps outside that selection.
  • CI for this exact head reports 510/510 corpus programs matching Ruby and a successful Linux build. I did not repeat the entire corpus or Linux validation locally; broader CI jobs remain pending.
  • git diff --check passed.

Comment thread m_eval_leaf.mere Resolved
Comment thread m_eval_leaf.mere Resolved
Comment thread tools/socket_glue.c Resolved

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed df48ca0 against the previously reviewed 637a705. Both previous Major findings are resolved. No new actionable findings.

  • TCP EOF now returns nil; actual payloads retain the [data, nil] shape.
  • Resolution expiry releases pending jobs while preserving resolved addresses and active connection attempts. The previous delayed-IPv6 reproduction now connects successfully over IPv4. The added failure-precedence case also matches Ruby.

Validation:

  • Fresh O2 build with the pinned Mere 991f299b compiler and mgz revision.
  • Both original reproductions now match Ruby 4.0.9 with indexing ON/OFF.
  • The updated bounded TCP regression matched Ruby in both index modes, normally and with MERE_RUBY_GC_STRESS=100.
  • Native resolver lifecycle checks passed, including strict compiler warnings, cancellation, fork, nonblocking connection flags, and local binding.
  • The repository's CRuby test runner reports 22/22 matching TCP tests, with zero failures/errors.
  • CI for this exact head reports 511/511 corpus programs matching Ruby and a successful Linux build. Broader jobs remain pending.
  • git diff --check passed.

The two previous findings can be considered addressed. Full corpus and Linux execution were not repeated locally.

Comment thread m_eval_leaf.mere Resolved
Comment thread m_eval_leaf.mere Resolved
@284km
284km marked this pull request as ready for review November 7, 2026 09:34
@284km
284km merged commit 97a5507 into main Nov 7, 2026
29 checks passed
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