From 63c586f6bb45d6825f2a2beef511c737698251b7 Mon Sep 17 00:00:00 2001 From: Dan Kubb Date: Thu, 23 Jul 2015 13:45:43 -0700 Subject: [PATCH] Change Mutant::Isolation.call to use block form of IO.pipe * The block form automatically closes the reader and writer if they have not already been closed. This prevents resource leakage if the method exits due to an exception. * Refactor specs to have common setup performed in a before block. --- lib/mutant/isolation.rb | 24 +++++++------ spec/unit/mutant/isolation_spec.rb | 58 +++++++++++++++--------------- 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/lib/mutant/isolation.rb b/lib/mutant/isolation.rb index 91a2d276..220a2e6d 100644 --- a/lib/mutant/isolation.rb +++ b/lib/mutant/isolation.rb @@ -38,23 +38,25 @@ module Mutant # # @api private def self.call(&block) - reader, writer = IO.pipe.map(&:binmode) + IO.pipe(binmode: true) do |reader, writer| + begin + pid = Process.fork do + File.open(File::NULL, 'w') do |file| + $stderr.reopen(file) + reader.close + writer.write(Marshal.dump(block.call)) + writer.close + end + end - pid = Process.fork do - File.open('/dev/null', 'w') do |file| - $stderr.reopen(file) - reader.close - writer.write(Marshal.dump(block.call)) writer.close + Marshal.load(reader.read) + ensure + Process.waitpid(pid) if pid end end - - writer.close - Marshal.load(reader.read) rescue => exception raise Error, exception - ensure - Process.waitpid(pid) if pid end end # Fork diff --git a/spec/unit/mutant/isolation_spec.rb b/spec/unit/mutant/isolation_spec.rb index e1c55104..819fa336 100644 --- a/spec/unit/mutant/isolation_spec.rb +++ b/spec/unit/mutant/isolation_spec.rb @@ -64,37 +64,39 @@ RSpec.describe Mutant::Isolation::Fork do end end - it 'uses primitives in correct order when fork succeeds' do - reader, writer = double('reader'), double('writer') - expect(IO).to receive(:pipe).ordered.and_return([reader, writer]) - expect(reader).to receive(:binmode).and_return(reader).ordered - expect(writer).to receive(:binmode).and_return(writer).ordered - pid = double('PID') - expect(Process).to receive(:fork).ordered.and_yield.and_return(pid) - file = double('file') - expect(File).to receive(:open).ordered.with('/dev/null', 'w').and_yield(file) - expect($stderr).to receive(:reopen).ordered.with(file) - expect(reader).to receive(:close).ordered - expect(writer).to receive(:write).ordered.with(Marshal.dump(:foo)) - expect(writer).to receive(:close).ordered - expect(writer).to receive(:close).ordered - expect(reader).to receive(:read).ordered.and_return(Marshal.dump(:foo)) - expect(Process).to receive(:waitpid).with(pid) + context 'uses primitives in correct order' do + let(:reader) { double('reader') } + let(:writer) { double('writer') } - expect(object.call { :foo }).to be(:foo) - end + before do + expect(IO).to receive(:pipe).with(binmode: true).ordered do |&block| + block.call([reader, writer]) + end + end - it 'uses primitives in correct order when fork fails' do - reader, writer = double('reader'), double('writer') - expect(IO).to receive(:pipe).ordered.and_return([reader, writer]) - expect(reader).to receive(:binmode).and_return(reader).ordered - expect(writer).to receive(:binmode).and_return(writer).ordered - expect(Process).to receive(:fork).ordered.and_return(nil) - expect(Process).to_not receive(:waitpid) + it 'when fork succeeds' do + pid = double('PID') + expect(Process).to receive(:fork).ordered.and_yield.and_return(pid) + file = double('file') + expect(File).to receive(:open).ordered.with('/dev/null', 'w').and_yield(file) + expect($stderr).to receive(:reopen).ordered.with(file) + expect(reader).to receive(:close).ordered + expect(writer).to receive(:write).ordered.with(Marshal.dump(:foo)) + expect(writer).to receive(:close).ordered + expect(writer).to receive(:close).ordered + expect(reader).to receive(:read).ordered.and_return(Marshal.dump(:foo)) + expect(Process).to receive(:waitpid).with(pid) - expect(writer).to receive(:close).ordered - expect(reader).to receive(:read).ordered.and_return(Marshal.dump(:foo)) - expect(object.call).to be(:foo) + expect(object.call { :foo }).to be(:foo) + end + + it 'when fork fails' do + expect(Process).to receive(:fork).ordered.and_return(nil) + expect(Process).to_not receive(:waitpid) + expect(writer).to receive(:close).ordered + expect(reader).to receive(:read).ordered.and_return(Marshal.dump(:foo)) + expect(object.call).to be(:foo) + end end end end