✅🚧 Add JRuby to CI

[nevans]

Fixes #454. (previous related PRs: #528, #470)

• assert_local_raised and assert_reraised
  • ✅🚧 Mark local stacktrace tests pending for JRuby #729
  • JRuby: raise should re-populate exception trace set to nil jruby/jruby#9528
  • Fixed by: [fix] re-raising exception with nil backtrace jruby/jruby#9548
• NoMatchingPatternError inheritance:
  • JRuby issue: NoMatchingPatternError#=== doesn't match instance of NoMatchingPatternKeyError jruby/jruby#9537
  • Fixed by: Fix inheritance of NoMatchingPatternKeyError jruby/jruby#9542
• String#% with Hash#default_proc
  • JRuby 10.0 issue: Kernel#sprintf calls Hash#default_proc with incorrect args jruby/jruby#9558
  • JRuby 10.1 issue: Kernel#sprintf calls Hash#default_proc with incorrect args jruby/jruby#9568
  • JRuby 10.0 fix: Pass correct args for Hash#default_proc jruby/jruby#9567
  • Ruby spec update: Expand Kernel#sprintf Hash default to include proc behavior jruby/jruby#9569
• Various flaky TLS tests (both STARTTLS and implicit TLS)
  • Hanging rather than raising OpenSSL::SSL::SSLError
    • JRuby issue: starttls test in net-imap hangs the suite jruby/jruby#9529
  • Raising #<IOError: Input record to big...> when it should succeed
  • JRuby-openssl bug when reads and writes to SSLSocket come from different threads
    • fixed by jruby/jruby-openssl@7b7421bb
    • fix released in jruby-openssl v0.19.0
    • Gemfile adds platform specific dependency for jruby-openssl >= 0.19.0
• Data issues
  • TEMP: bring back Data polyfill, behind RUBY_ENGINE check and compatibility probe
  • TEMP: need to add Psych.load_tags for Data polyfill classes
  • JRuby issue: Data subclasses in net-imap are losing fields jruby/jruby#9530
• test_connection_closed_without_greeting depends on ObjectSpace.each_object
  • TEMP: omit on JRuby when ObjectSpace.each_object isn't enabled
  • This test should probably be updated to avoid ObjectSpace.each_object
• closed connection raises IOError instead of InvalidTaggedResponseError
  • This is probably related to threading: the receiver thread raises the InvalidTaggedResponseError and closes the socket. The client thread may be attempting to use the socket before checking @exception, or it may be reading @exception unsynchronized. This may also need better synchronization in the test code.

[headius]

Hey checking back in after a long time. I'd like to help get this green... have you looked into the failures at all?

[nevans]

@headius Thanks!

I haven't looked into it recently, beyond rebasing the branch and verifying it still had the same issues... the tests that (reliably) fail in CI are all passing locally for me. That certainly slows down my debugging ability.

I don't remember for sure, but I think that some of the other tests that are already omitted or pending in this branch may also pass locally (or maybe they're flaky?). I'd prefer to fix all of them and omit none: omitting every test that actually makes a connection doesn't give me much confidence. But, for the ratchet effect, getting JRuby into CI with a bunch of critical tests omitted is probably better than not testing it at all.

The following is going off of memory the last time I looked into this... and I didn't have time to fully test my hypothesis, so I may be completely wrong, buuuut (caveat's aside)... I think the biggest source of test failures are exceptions from IO#close. My interpretation of the CRuby IO#close code is that I don't think it can raise that exception.

When IO#close is called, rb_io_close_m checks (with the GVL) whether or not it's already closed. And if not, io_close_fptr goes ahead and closes it, without dropping the GVL (as far as I can tell) or adding itself to the list of threads that are blocking on it. At this point, (if it were to release the GVL) if any other thread attempts to close the IO, it would just return immediately without exception. After that, it notifies any other threads that are blocking on that IO to raise that exception.

But I might be misunderstanding what the code does and biased by what I want it to do. 😉 I found some issues in the b.r-l.o tracker that led me to think maybe it didn't or doesn't work the way I assume. So, I looked to see if there was a test or a spec, and there was (and I think it was written by you, so thanks!). But (as far as I can tell) the spec for this doesn't distinguish between an error that's raised by IO#close or an error that's caused by IO#close and raised by another thread that's blocked on IO#close.

  it 'raises an IOError with a clear message' do
    matching_exception = nil

    -> do
      IOSpecs::THREAD_CLOSE_RETRIES.times do
        read_io, write_io = IO.pipe
        going_to_read = false

        thread = Thread.new do
          begin
            going_to_read = true
            read_io.read
          rescue IOError => ioe
            if ioe.message == IOSpecs::THREAD_CLOSE_ERROR_MESSAGE
              matching_exception = ioe
            end
            # try again
          end
        end

        # best attempt to ensure the thread is actually blocked on read
        Thread.pass until going_to_read && thread.stop?
        sleep(0.001)

        read_io.close    # <--------------------------- does this raise the exception?
        thread.join
        write_io.close   # <--------------------------- does this raise the exception?

        matching_exception&.tap {|ex| raise ex} # <---- or does this raise the exception?
      end
    end.should raise_error(IOError, IOSpecs::THREAD_CLOSE_ERROR_MESSAGE)
  end

My hypothesis is that CRuby's IO#close can't raise this exception, but JRuby's can. I'd like to update the spec to make that distinction. But... I had to switch to other projects before I could test this hypothesis.

With all of that said, even if JRuby's IO#close does behaves differently, it's highly likely that there's still threading bugs in the tests or in the client that need to be fixed. We can still improve the code to not trigger that condition, not just as a workaround but as an improvement that may fix other issues too.

[nevans]

Okay, so looking into it just now, I think my memory or interpretation (or both!) was off by a bit: any blocking threads are notified before the file descriptor is completely cleared. That does introduce the possibility that multiple threads could call IO#close while waiting for the blocking threads to be notified. And, the fiber scheduler also gets to play a role in this, which opens up even more possibilities.

Nevertheless, it still seems to me that, the closer doesn't add itself blocking list for that IO, so it should never be interrupted with that exception. And, after all of the (other) blocking threads have been removed from the blocking list, the closer thread gets to do its thing while keeping the GVL.

Regardless, I'll repeat myself on this:

even if JRuby's IO#close does behaves differently, it's highly likely that there's still threading bugs in the tests or in the client that need to be fixed. We can still improve the code to not trigger that condition, not just as a workaround but as an improvement that may fix other issues too.

[kares]

FYI: the latest (flaky) test failure:

Failure: test_imaps_with_ca_file(IMAPTest):
  Exception raised:
  <#<Java::JavaNio::BufferOverflowException: >>
  Backtrace:
    java.base/java.nio.ByteBuffer.put(ByteBuffer.java:1038)
    java.base/sun.nio.ch.IOUtil.write(IOUtil.java:97)
    java.base/sun.nio.ch.IOUtil.write(IOUtil.java:58)
    java.base/sun.nio.ch.SocketChannelImpl.write(SocketChannelImpl.java:542)
    org.jruby.ext.openssl.SSLSocket$JavaSocketChannel.write(SSLSocket.java:1296)
    org.jruby.ext.openssl.SSLSocket.writeToChannel(SSLSocket.java:652)
    org.jruby.ext.openssl.SSLSocket.flushData(SSLSocket.java:640)
    org.jruby.ext.openssl.SSLSocket.sysreadImpl(SSLSocket.java:838)
    org.jruby.ext.openssl.SSLSocket.sysread(SSLSocket.java:887)
    home.runner.$_dot_rubies.jruby_minus_head.lib.ruby.stdlib.openssl.buffering.️❤ def fill_rbuff #0(/home/runner/.rubies/jruby-head/lib/ruby/stdlib/openssl/buffering.rb:80)

is already fixed upstream in jruby-openssl >= 0.19.0