Run tests on TruffleRuby, all tests pass now (#2198)

* Support skip_on :truffleruby

* Remove unused variable declaration

* Properly skip tests which need fork

* Improve NO_FORK_MSG

* Keep the Tempfile instances alive in test_redirect_io.rb

* Otherwise they could GC in the middle of the test, and the files could
  then be deleted.

* Use a better way to find a free port

* Read directly from the socket in #read_and_drop

* There is no point to decode the bytes since we are closing the socket
  in Puma::MiniSSL::Socket#close.
* Also, calling #engine_read_all might cause further SSL errors, which
  could hide the first SSL error. This notably happens in
  TestPumaServerSSLClient#test_verify_fail_if_no_client_cert
  if the server is faster than the client. The error in that case is
  "System error: Success - 0 (Puma::MiniSSL::SSLError)" which is not
  actually an error, but there is also nothing to read further from SSL.

* TruffleRuby should pass the CI now, remove from allowed failures

* Use a timeout of 120 for all non-MRI implementations

* 60 doesn't seem enough in CI for TestThreadPool#test_trim on TruffleRuby.

* Fix check for cluster mode in integration tests

* Improve integration tests to fail more clearly if the pid file does not exist

* Make integration tests more robust

* Add skips for unreliable or racy tests

* Add ChangeLog entry

* No need to run RuboCop on non-MRI implementations

* This should speed up CI a bit for those jobs.

.github/workflows/ruby.yml

@@ -18,7 +18,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ ubuntu-18.04, macos ]
-        ruby: [ 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, head, jruby ]
+        ruby: [ 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, head, jruby, truffleruby-head ]
 
     steps:
       - name: repo checkout
@@ -42,9 +42,13 @@ jobs:
       - name: compile
         run:  bundle exec rake compile
 
+      - name: rubocop
+        if: startsWith(matrix.ruby, '2.')
+        run: bundle exec rake rubocop
+
       - name: test
         timeout-minutes: 8
-        run: bundle exec rake
+        run: bundle exec rake test:all
 
   win32:
     name: >-
@@ -89,7 +93,7 @@ jobs:
         timeout-minutes: 8
         run: bundle exec rake
 
-  nonMRIHead:
+  allowedFailures:
     name: >-
       ${{ matrix.cfg.os }}, ${{ matrix.cfg.ruby }}
     env:
@@ -104,8 +108,7 @@ jobs:
       fail-fast: false
       matrix:
         cfg:
-          - { os: ubuntu-latest, ruby: jruby-head       }
+          - { os: ubuntu-latest, ruby: jruby-head }
-          - { os: ubuntu-latest, ruby: truffleruby-head }
 
     steps:
       - name: repo checkout

History.md

@@ -25,6 +25,7 @@
   * Rescue IO::WaitReadable instead of EAGAIN for blocking read (#2121)
   * Ensure `BUNDLE_GEMFILE` is unspecified in workers if unspecified in master when using `prune_bundler` (#2154)
   * Rescue and log exceptions in hooks defined by users (on_worker_boot, after_worker_fork etc) (#1551)
+  * Read directly from the socket in #read_and_drop to avoid raising further SSL errors (#2198)
   
 * Refactor
   * Remove unused loader argument from Plugin initializer (#2095)

lib/puma/minissl.rb

@@ -125,11 +125,14 @@ module Puma
 
       def read_and_drop(timeout = 1)
         return :timeout unless IO.select([@socket], nil, nil, timeout)
-        return :eof unless read_nonblock(1024)
+        case @socket.read_nonblock(1024, exception: false)
-        :drop
+        when nil
-      rescue Errno::EAGAIN
+          :eof
-        # do nothing
+        when :wait_readable
-        :eagain
+          :eagain
+        else
+          :drop
+        end
       end
 
       def should_drop_bytes?

test/helper.rb

@@ -43,15 +43,10 @@ def hit(uris)
 end
 
 module UniquePort
-  @port  = 3211
-  @mutex = Mutex.new
-
   def self.call
-    @mutex.synchronize {
+    TCPServer.open('127.0.0.1', 0) do |server|
-      @port += 1
+      server.connect_address.ip_port
-      @port = 3307 if @port == 3306  # MySQL on Actions
+    end
-      @port
-    }
   end
 end
 
@@ -62,7 +57,7 @@ module TimeoutEveryTestCase
   end
 
   def run(*)
-    ::Timeout.timeout(Puma.jruby? ? 120 : 60, TestTookTooLong) { super }
+    ::Timeout.timeout(RUBY_ENGINE == 'ruby' ? 60 : 120, TestTookTooLong) { super }
   end
 end
 
@@ -78,7 +73,7 @@ module TestSkips
   # usage: skip NO_FORK_MSG unless HAS_FORK
   # windows >= 2.6 fork is not defined, < 2.6 fork raises NotImplementedError
   HAS_FORK = ::Process.respond_to? :fork
-  NO_FORK_MSG = "Kernel.fork isn't available on the #{RUBY_PLATFORM} platform"
+  NO_FORK_MSG = "Kernel.fork isn't available on #{RUBY_ENGINE} on #{RUBY_PLATFORM}"
 
   # socket is required by puma
   # usage: skip UNIX_SKT_MSG unless UNIX_SKT_EXIST
@@ -99,11 +94,11 @@ module TestSkips
   # optional suffix kwarg is appended to the skip message
   # optional suffix bt should generally not used
   def skip_on(*engs, suffix: '', bt: caller)
-    skip_msg = false
     engs.each do |eng|
       skip_msg = case eng
         when :darwin   then "Skipped on darwin#{suffix}"    if RUBY_PLATFORM[/darwin/]
         when :jruby    then "Skipped on JRuby#{suffix}"     if Puma.jruby?
+        when :truffleruby then "Skipped on TruffleRuby#{suffix}" if RUBY_ENGINE == "truffleruby"
         when :windows  then "Skipped on Windows#{suffix}"   if Puma.windows?
         when :ci       then "Skipped on ENV['CI']#{suffix}" if ENV["CI"]
         when :no_bundler then "Skipped w/o Bundler#{suffix}"  if !defined?(Bundler)

test/shell/run.rb

@@ -1,10 +1,10 @@
 require "puma"
 require "puma/detect"
 
-TESTS_TO_RUN = if Puma.jruby?
+TESTS_TO_RUN = if Process.respond_to?(:fork)
-  %w[t1 t2]
-else
   %w[t1 t2 t3]
+else
+  %w[t1 t2]
 end
 
 results = TESTS_TO_RUN.map do |test|

test/shell/t1.rb

@@ -1,8 +1,8 @@
 system "ruby -rrubygems -Ilib bin/puma -p 10102 -C test/shell/t1_conf.rb test/rackup/hello.ru &"
-sleep (defined?(JRUBY_VERSION) ? 7 : 5)
-system "curl http://localhost:10102/"
 
-system "kill `cat t1-pid`"
+sleep 1 until system "curl http://localhost:10102/"
+
+Process.kill :TERM, Integer(File.read("t1-pid"))
 
 sleep 1
 

test/shell/t2.rb

@@ -1,6 +1,6 @@
 system "ruby -rrubygems -Ilib bin/pumactl -F test/shell/t2_conf.rb start &"
-sleep (defined?(JRUBY_VERSION) ? 7 : 5)
+
-system "curl http://localhost:10103/"
+sleep 1 until system "curl http://localhost:10103/"
 
 out=`ruby -rrubygems -Ilib bin/pumactl -F test/shell/t2_conf.rb status`
 

test/shell/t3.rb

@@ -3,13 +3,13 @@ sleep 5
 
 worker_pid_was_present = File.file? "t3-worker-2-pid"
 
-system "kill `cat t3-worker-2-pid`" # kill off a worker
+Process.kill :TERM, Integer(File.read("t3-worker-2-pid")) # kill off a worker
 
 sleep 2
 
 worker_index_within_number_of_workers = !File.file?("t3-worker-3-pid")
 
-system "kill `cat t3-pid`"
+Process.kill :TERM, Integer(File.read("t3-pid"))
 
 File.unlink "t3-pid" if File.file? "t3-pid"
 File.unlink "t3-worker-0-pid" if File.file? "t3-worker-0-pid"

test/test_integration_pumactl.rb

@@ -20,7 +20,7 @@ class TestIntegrationPumactl < TestIntegration
   end
 
   def test_stop_tcp
-    skip_on :jruby # Undiagnose thread race. TODO fix
+    skip_on :jruby, :truffleruby # Undiagnose thread race. TODO fix
     @control_tcp_port = UniquePort.call
     cli_server "-q test/rackup/sleep.ru --control-url tcp://#{HOST}:#{@control_tcp_port} --control-token #{TOKEN} -S #{@state_path}"
 

test/test_redirect_io.rb

@@ -7,8 +7,11 @@ class TestRedirectIO < TestIntegration
   def setup
     super
 
-    @out_file_path = Tempfile.new('puma-out').path
+    # Keep the Tempfile instances alive to avoid being GC'd
-    @err_file_path = Tempfile.new('puma-err').path
+    @out_file = Tempfile.new('puma-out')
+    @err_file = Tempfile.new('puma-err')
+    @out_file_path = @out_file.path
+    @err_file_path = @err_file.path
   end
 
   def teardown
@@ -16,6 +19,8 @@ class TestRedirectIO < TestIntegration
 
     paths = [@out_file_path, @err_file_path, @old_out_file_path, @old_err_file_path].compact
     File.unlink(*paths)
+    @out_file = nil
+    @err_file = nil
   end
 
   def test_sighup_redirects_io_single
@@ -47,7 +52,7 @@ class TestRedirectIO < TestIntegration
   end
 
   def test_sighup_redirects_io_cluster
-    skip_on :jruby # Server isn't coming up in CI, TODO Fix
+    skip NO_FORK_MSG unless HAS_FORK
     skip_unless_signal_exist? :HUP
 
     cli_args = [

test/test_thread_pool.rb

@@ -64,7 +64,7 @@ class TestThreadPool < Minitest::Test
   end
 
   def test_trim
-    skip_on :jruby # Undiagnose thread race. TODO fix
+    skip_on :jruby, :truffleruby # Undiagnose thread race. TODO fix
     pool = new_pool(0, 1) do |work|
       @work_mutex.synchronize do
         @work_done.signal
@@ -87,6 +87,7 @@ class TestThreadPool < Minitest::Test
   end
 
   def test_trim_leaves_min
+    skip_on :truffleruby # Undiagnose thread race. TODO fix
     pool = new_pool(1, 2) do |work|
       @work_mutex.synchronize do
         @work_done.signal