Skip to content

socket: open with a block must close, which is 27 of the hangs in #916 - #932

Merged
tannevaled merged 3 commits into
mainfrom
socket-open-block
Nov 14, 2026
Merged

tannevaled merged 3 commits into
mainfrom
socket-open-block

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

socket: open with a block must close, which is 27 of the hangs in #916

TCPSocket.open, TCPServer.open and Socket.open were bare aliases for
.new, so the block form closed nothing: the socket outlived the block and the
peer waited for an EOF that never came.

⛔ That is the whole net-http column of issue 916

library/net-http/http/fixtures/http_server.rb serves one client at a time
and shuts down by opening a new connection to write an unterminated "CLOSE":

def close
  TCPSocket.open(ip, port) { |socket| socket.write "CLOSE" }
  @thread.join
end

With that socket still open the server's gets("\r\n\r\n") never returns, the
listen loop never comes back round to accept, and @thread.join never
completes. Every one of the 27 library/net-http files ends on that join.

Measured, against the restored MRI oracle

The ten-line reproduction, before:

MRI 4.0.6 rbgo
closed? after the block true false
the peer's gets "CLOSE" blocks for ever
join :joined hangs

After this change rbgo prints the same seven lines as MRI, in the same order,
and exits 0.

And over library/net-http + library/socket + library/timeout, 321 files,
30 s each:

files killed under rbgo passing
before 36 243
after 9 274

27 hangs gone, +31 examples. The nine that remain are the other two columns
of that issue, untouched here: library/socket ×8 and library/timeout ×1.
(net-http/http/start_spec.rb is now killed on the control — MRI hangs on
it and rbgo passes 12 — so the net-http column is 27 → 0.)

⛔ It is not the GVL, which is what I expected to find

A goroutine dump of a hung get_spec.rb puts the server thread correctly
inside ioBlock with the GVL released (readUntil → bufio.ReadByte →
net.(*conn).Read) and the main thread in threadJoin. The emulated GVL
behaves; the socket was simply still open.

Nor does Net::HTTP leak its own connection — my first guess, and wrong: the
reproduction shows peer closed (gets returned nil) after the request under
both engines.

How it closes, and why there is no fourth spelling of close

It sends the class's own close method, so TCPSocket, TCPServer and
Socket each keep the single close they already had. Each is guarded by
s.closed and therefore idempotent, so a block that closed the socket itself
is not closed twice. The close runs in a defer, so an exception or a break
leaves it closed too: this is an ensure, not a tidy-up on the happy path.

The test fails rather than hangs

It asserts the contract on the socket the block was handed, not through a
peer: the block's value comes back (not the socket), closed? is true after a
normal return and after an exception, and open without a block still gives
an open socket exactly as .new does. A regression therefore fails this test
instead of hanging it.

Refs #931, #916

(Both stay open: #931 also asks for an audit of the same block form on
UNIXSocket and UDPSocket, and for gets(sep) returning partial data at EOF,
neither of which this change settles; #916 still has nine hangs left.)

🤖 Generated with Claude Code



TCPSocket.open, TCPServer.open and Socket.open were bare aliases for .new, so
the block form closed nothing: the socket outlived the block and the peer
waited for an EOF that never came.

⛔ THAT IS THE WHOLE net-http COLUMN OF #916. ruby/spec's
library/net-http/http/fixtures/http_server.rb serves ONE client at a time and
shuts down by opening a NEW connection to write an unterminated "CLOSE":

    def close
      TCPSocket.open(ip, port) { |socket| socket.write "CLOSE" }
      @thread.join
    end

With that socket still open the server's `gets("\r\n\r\n")` never returns, the
listen loop never comes back to accept, and @thread.join never completes. All
27 library/net-http files end on that join.

Measured against the restored MRI oracle, the same ten-line reproduction:

    closed? after the block     MRI true    rbgo false
    the peer's gets             MRI "CLOSE" rbgo blocks for ever
    join                        MRI :joined rbgo hangs

and after this change rbgo prints the same seven lines as MRI, in the same
order, and exits 0.

⛔ IT IS NOT THE GVL, which is what I expected to find. A goroutine dump of a
hung get_spec.rb puts the server thread correctly INSIDE ioBlock with the GVL
released (readUntil -> bufio.ReadByte -> net.(*conn).Read) and the main thread
in threadJoin. The emulated GVL behaves; the socket was simply still open. Nor
does Net::HTTP leak its own connection: the reproduction shows "peer closed"
after the request under BOTH engines.

HOW IT CLOSES, and why there is no fourth spelling of close: it sends the
CLASS'S OWN `close` method, so TCPSocket, TCPServer and Socket each keep the
one close they already had -- each guarded by `s.closed`, hence idempotent, so
a block that closed the socket itself is not closed twice. The close runs in a
defer, so an exception or a break leaves the socket closed as well: this is an
ensure, not a tidy-up on the happy path.

The test asserts the contract on the socket the block was handed rather than
through a peer, so a regression FAILS it instead of HANGING it: the block's
value comes back (not the socket), closed? is true after a normal return and
after an exception, and `open` with no block still answers with an open
socket exactly as .new does.

Refs #931, #916

Co-Authored-By: Claude Opus 5 <[email protected]>
…ix exposed

The ruby/spec ratchet refused the previous commit with +27 examples overall
and ONE file down:

    passing examples: 23699 (baseline 23672, +27)
    -- REGRESSED (1) --
       core/io/nonblock_spec.rb: 3 passing, baseline 4 (-1)

⛔ The gate was right to refuse, and nothing had regressed. The example is

    TCPServer.open(0) { |socket| socket.nonblock?.should == true }

and while `open` ignored its block that body NEVER RAN: the example passed with
no assertion evaluated, and the baseline recorded that pass. Honouring the
block made it run, and it raised NoMethodError — TCPServer#nonblock? does not
exist. A test that had never run started running.

Measured both ways rather than assumed, because the +27 includes other work
merged into main today and the -1 could have been someone else's:

    core/io/nonblock_spec.rb, main ba89ba6:  pass=4 fail=0 error=0
    core/io/nonblock_spec.rb, this branch:    pass=3 fail=0 error=1
      NoMethodError  undefined method 'nonblock?' for an instance of TCPServer

So it was mine.

IO#nonblock? is installed on IO alone and reads an *IOObj field, which a socket
is not. This installs `nonblock?` on the socket classes FROM THE SAME
`io/nonblock` feature hook, so one place decides what that require adds and the
two cannot drift; the socket classes exist from startup, so they are there when
it fires.

The answer is `true` because it is true here: Go puts every socket descriptor
in non-blocking mode and polls it through the runtime, which is also why MRI
reports true for a socket by default.

⛔ `nonblock=` is deliberately absent. The blocking mode of these descriptors
belongs to the Go runtime, and a writer that accepted `false` and changed
nothing would be a more expensive lie than a missing method. MRI does allow it;
the divergence is stated here rather than papered over.

The file is back to 4 passing — but the fourth example now RUNS and asserts,
where on main it passes with nothing evaluated. Same number, different meaning.

Refs #931

Co-Authored-By: Claude Opus 5 <[email protected]>
The coverage ratchet named installSocketNonblock at 83.3%, newly below 100%.
The uncovered statement was the `continue`:

    cls, ok := vm.consts[name].(*RClass)
    if !ok {
        continue          // ← all six classes exist, so nothing reaches this
    }

All six socket classes are registered at startup, so that arm is unreachable on
this build and no test can honestly cover it. Writing the guard as

    if cls, ok := vm.consts[name].(*RClass); ok { … }

keeps exactly the same protection — a name that is not a class is skipped
rather than type-asserted into a panic — and has no statement on the false
path to explain away.

⛔ The alternative was a test that deletes a class from vm.consts to drive the
`continue`. That buys the percentage by asserting a state the VM never has, and
leaves the next reader wondering which of the two is the lie. Removing the dead
statement is cheaper and truer.

Measured before pushing, with covratchet -require (#925), which is what that
flag exists for after two commits in one day failed to move this same number:

    required 2 function(s), matched 2 record(s) in a profile of 7351
    all required functions at 100%

The ruby/spec ratchet is already green on this head — the nonblock commit
settled core/io/nonblock_spec.rb — so this is the last red.

Refs #931

Co-Authored-By: Claude Opus 5 <[email protected]>
@tannevaled
tannevaled merged commit 6b51deb into main Nov 14, 2026
26 checks passed
@tannevaled
tannevaled deleted the socket-open-block branch November 14, 2026 06:43
tannevaled added a commit that referenced this pull request Nov 14, 2026
#932 merged while this was in review, so the table's net-http row said 27
as the CURRENT figure for a module that no longer hangs at all. Publishing
that would have been the same defect this pull request exists to correct.

Re-measured on main at 6b51deb, 321 files across library/net-http +
library/socket + library/timeout, MRI 4.0.6 as the control, 30 s per file:
275 passing against MRI's 371, NINE killed under rbgo and none under MRI.

So the tally is 75 -> 9, and the nine are the last two rows: Thread#kill
cannot interrupt a thread blocked in a native call (#933).

Co-Authored-By: Claude Opus 5 <[email protected]>
tannevaled added a commit that referenced this pull request Nov 14, 2026
#934)

* README: the Net::FTP hang figures describe code that has since changed

The map of where the work is said:

    The largest single debt is a hang, not a missing feature. Net::FTP alone
    is 442 examples behind, and 39 of its 56 spec files have to be killed by
    the clock on rbgo and none on MRI (#916).

Re-measured today against MRI 4.0.6, same corpus, 30 s per file, on main at
ba89ba6: Net::FTP is 128 behind (459 -> 331) and NO file is killed at all.
Only the figures this measurement actually covers are touched; the whole-corpus
numbers above that bullet are left exactly as they were, because I have not
re-swept the corpus.

The replacement states the thing that took a day to establish and that a
single number hides: ONE SYMPTOM, TWO UNRELATED CAUSES. Half the files hung
because a blocking socket call held the emulated GVL (#924); the other half
because TCPSocket.open with a block never closed the socket, so the peer waited
for an EOF that never came (#932, open). Neither fix moves the other's files,
which is why the first one looked like it had done nothing when it was measured
against the second one's specs.

The table also carries what is NOT fixed, with its cause named rather than left
as a count: library/socket 8 and library/timeout 1, both because Thread#kill
cannot interrupt a thread blocked in a native call (#933). A reader should be
able to tell the fixed from the merely counted.

⛔ And the note that outlives all of these numbers: none of it was measurable
until the reference interpreter was repaired. It had been scoring a clean 0 on
every file because its bundled gems were off the load path, and a sweep reports
that as a RESULT -- 56 files of "control 0 passing" reads as the implementation
under test beating the reference. cmd/mriref (#930) is named so the next person
measuring starts from a working oracle.

Co-Authored-By: Claude Opus 5 <[email protected]>

* README: net-http is now 0 too, measured on the merged main

#932 merged while this was in review, so the table's net-http row said 27
as the CURRENT figure for a module that no longer hangs at all. Publishing
that would have been the same defect this pull request exists to correct.

Re-measured on main at 6b51deb, 321 files across library/net-http +
library/socket + library/timeout, MRI 4.0.6 as the control, 30 s per file:
275 passing against MRI's 371, NINE killed under rbgo and none under MRI.

So the tally is 75 -> 9, and the nine are the last two rows: Thread#kill
cannot interrupt a thread blocked in a native call (#933).

Co-Authored-By: Claude Opus 5 <[email protected]>

---------

Co-authored-by: Claude Opus 5 <[email protected]>
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