Fix stuck isolation

This was both an MRI and a mutant problem, and its likely that the
underlying MRI problem can still manifest itself. Its now simpy just
significantly more unlikely.

A bit of context:

* Mutant is and was waiting for an FD to become readable via IO.select.
* Mutant did NOT "finish" reading the FD per IO.select return, instead
  it would only read upto 4096 bytes, even if the FD could provide more
  input. Mutant would loop around to call another IO.select on the FD.
* MRI does internally maintain a buffer before calling `read()` on the
  underlying file descriptor. Mutant often would per IO.select only hit
  that buffer, not the underlying FD.
* MRI can "forget" the Ruby side IO#read_nonblocking should not block,
  so when the buffer is exhausted it may call `read()` in blocking mode.
* Mutant triggered this (potentially buggy MRI mechanism) very often.

Changelog.md

@@ -1,4 +1,10 @@
-# v0.10.16 2020-12-09
+# v0.10.17 2020-12-09
+
+* Fix low frequency stuck isolation reads.
+
+  [#1146](https://github.com/mbj/mutant/pull/1146)
+
+# v0.10.16 2020-12-08
 
 * Minor performance improvements on small runs.
 

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    mutant (0.10.16)
+    mutant (0.10.17)
       abstract_type (~> 0.0.7)
       adamantium (~> 0.2.0)
       anima (~> 0.3.1)

lib/mutant/isolation/fork.rb

@@ -64,6 +64,7 @@ def parent
         end
       end # Pipe
 
+      # rubocop:disable Metrics/ClassLength
       class Parent
         include(
           Anima.new(*ATTRIBUTES),
@@ -149,17 +150,26 @@ def read_targets(targets)
 
             break unless ready
 
-            ready.each do |fd|
-              if fd.eof?
-                targets.delete(fd)
+            ready.each do |target|
+              if target.eof?
+                targets.delete(target)
               else
-                targets.fetch(fd) << fd.read_nonblock(READ_SIZE)
+                read_fragment(target, targets.fetch(target))
               end
             end
           end
         end
         # rubocop:enable Metrics/MethodLength
 
+        def read_fragment(target, fragments)
+          loop do
+            result = target.read_nonblock(READ_SIZE, exception: false)
+            break unless result.instance_of?(String)
+            fragments << result
+            break if result.bytesize < READ_SIZE
+          end
+        end
+
         # rubocop:disable Metrics/MethodLength
         def terminate_graceful
           status = nil
@@ -199,6 +209,7 @@ def add_result(result)
           @result = defined?(@result) ? @result.add_error(result) : result
         end
       end # Parent
+      # rubocop:enable Metrics/ClassLength
 
       class Child
         include(

lib/mutant/version.rb

@@ -2,5 +2,5 @@
 
 module Mutant
   # Current mutant version
-  VERSION = '0.10.16'
+  VERSION = '0.10.17'
 end # Mutant

spec/unit/mutant/isolation/fork_spec.rb

@@ -77,7 +77,7 @@ def read(io, result)
     {
       receiver:  io,
       selector:  :read_nonblock,
-      arguments: [read_bytes],
+      arguments: [read_bytes, { exception: false }],
       reaction:  { return: result }
     }
   end
@@ -195,7 +195,7 @@ def apply
       ]
     end
 
-    context 'happy paths without configured timeouts' do
+    context 'without configured timeouts' do
       let(:raw_expectations) do
         [
           *prefork_expectations,
@@ -208,17 +208,80 @@ def apply
         ]
       end
 
-      it 'returns success result' do
-        verify_events do
-          expect(apply).to eql(
-            described_class::Result.new(
-              log:            log_fragment,
-              exception:      nil,
-              process_status: child_status_success,
-              timeout:        nil,
-              value:          block_return
+      context 'read results in wait error' do
+        let(:read_fragments) do
+          [
+            select([log_reader, result_reader], [log_reader, result_reader]),
+            eof(log_reader, false),
+            read(log_reader, :wait_readable),
+            eof(result_reader, false),
+            read(result_reader, result_fragment),
+            select([log_reader, result_reader], [log_reader, result_reader]),
+            eof(log_reader, true),
+            eof(result_reader, true)
+          ]
+        end
+
+        it 'returns success result' do
+          verify_events do
+            expect(apply).to eql(
+              described_class::Result.new(
+                log:            '',
+                exception:      nil,
+                process_status: child_status_success,
+                timeout:        nil,
+                value:          block_return
+              )
             )
-          )
+          end
+        end
+      end
+
+      context 'multiple reads' do
+        let(:full_fragment) { '_' * read_bytes }
+
+        let(:read_fragments) do
+          [
+            select([log_reader, result_reader], [log_reader, result_reader]),
+            eof(log_reader, false),
+            read(log_reader, full_fragment),
+            read(log_reader, log_fragment),
+            eof(result_reader, false),
+            read(result_reader, result_fragment),
+            select([log_reader, result_reader], [log_reader, result_reader]),
+            eof(log_reader, true),
+            eof(result_reader, true)
+          ]
+        end
+
+        it 'returns success result' do
+          verify_events do
+            expect(apply).to eql(
+              described_class::Result.new(
+                log:            full_fragment + log_fragment,
+                exception:      nil,
+                process_status: child_status_success,
+                timeout:        nil,
+                value:          block_return
+              )
+            )
+          end
+        end
+      end
+
+      context 'happy path' do
+        it 'returns success result' do
+          verify_events do
+            expect(apply).to eql(
+              described_class::Result.new(
+                log:            log_fragment,
+                exception:      nil,
+                process_status: child_status_success,
+                timeout:        nil,
+                value:          block_return
+              )
+            )
+          end
         end
       end
     end