GeezerVison

This is the one discovered during #9716

A litte background here:

This simple server logic is now sprinkled over various parts of the CRuby test suite after the WEBrick library got moved out of the default stdlib. Formerly, WEBrick was used to manage these test servers, and it has much more robust logic for starting up and shutting down servers. I've never been quite satisfied with the new tests.

There is an example of the teardown you would want in test/mri/net/http/utils.rb — @thread.kill followed by @thread.join — and it still fails intermittently on JRuby. Two tests errored that way on #9718's own CI run, TestNetHTTPKeepAlive#test_keep_alive_get_auto_retry and TestNetHTTP_v1_2#test_get__implicit_start, both Errno::EBADF raised from IO#close at line 35, the socket&.close in the ensure inside the accept loop. The thread is killed, unwinds into that ensure, the close raises, and join re-raises it into the test. Java 21 and Java 26 failed; Java 25 passed on the same commit.

That may be the better case to reason from, since the kill and join are already there, so nothing can be blamed on their absence.

The teardown — @thread&.kill then @thread&.join:

The accept loop, with the socket&.close that raises at L35:

Reported JRuby issue as #9722.

This one is separate: the open-uri teardown closes the server socket under a blocked accept with no kill, which raises on CRuby too — the helper's assumption, not an engine difference.

@aminmansuri Clarify for me what's left to do here... with the EBADF fix from #9723 do we still need this change?

Yes, still needed. They fix different things.

#9723 fixed close after a thread is killed. This PR is about a server that is never killed: shutdown closes the socket while the thread is still waiting in accept. That raises IOError, on CRuby too, and the thread's rescue doesn't catch it, so the test fails.

The fix does what CRuby does here: kill the thread first, then close.

looking at the code should I put a

@thread.join(10)

To prevent a weird hang?

If no timeout it could, under some very rare circumstances block forever.

Ok, then we're back to my original problem with this change: why do we need to do it this way and CRuby just does

  def shutdown
    @thread.kill
    @server.close
  end

These tests are synced from CRuby (which syncs with ruby/openuri, see https://github.com/ruby/open-uri/blob/master/test/open-uri/utils.rb#L29-L32) so any changes we make in our copy can easily get wiped out by future updates.

@aminmansuri If you're still out there I'm still interested in resolving this. The ideal situation is to be able to use CRuby's test without modifications, but only if the setup+teardown it includes is expected to work properly on JRuby. If there's a problem with the test, we need to upstream that along with our local changes, so those changes don't get wiped out.

I'm traveling right now so I can't get to it for a while.