[GR-16973] [GR-16975] Fix IO#write to not modify the argument when the encoding does not match.

PullRequest: truffleruby/934

Author: eregon

CHANGELOG.md

@@ -5,6 +5,7 @@ Bug fixes:
 * Set `RbConfig::CONFIG['ruby_version']` to the same value as the TruffleRuby version. This fixes reusing C extensions between different versions of TruffleRuby with Bundler (#1715).
 * Fixed `Symbol#match` returning `MatchData` (#1706).
 * Allow `Time#strftime` to be called with binary format strings.
+* Do not modify the argument passed to `IO#write` when the encoding does not match (#1714).
 
 Compatibility:
 

spec/ruby/core/io/flush_spec.rb

@@ -5,4 +5,33 @@
   it "raises IOError on closed stream" do
     lambda { IOSpecs.closed_io.flush }.should raise_error(IOError)
   end
+
+  describe "on a pipe" do
+    before :each do
+      @r, @w = IO.pipe
+    end
+
+    after :each do
+      @r.close
+      begin
+        @w.close
+      rescue Errno::EPIPE
+      end
+    end
+
+    # [ruby-core:90895] MJIT worker may leave fd open in a forked child.
+    # For instance, MJIT creates a worker before @r.close with fork(), @r.close happens,
+    # and the MJIT worker keeps the pipe open until the worker execve().
+    # TODO: consider acquiring GVL from MJIT worker.
+    guard_not -> { defined?(RubyVM::MJIT) && RubyVM::MJIT.enabled? } do
+      it "raises Errno::EPIPE if sync=false and the read end is closed" do
+        @w.sync = false
+        @w.write "foo"
+        @r.close
+
+        -> { @w.flush }.should raise_error(Errno::EPIPE, /Broken pipe/)
+        -> { @w.close }.should raise_error(Errno::EPIPE, /Broken pipe/)
+      end
+    end
+  end
 end

spec/ruby/core/io/shared/write.rb

@@ -69,6 +69,16 @@
     lambda { IOSpecs.closed_io.send(@method, "hello") }.should raise_error(IOError)
   end
 
+  it "does not modify the passed argument" do
+    File.open(@filename, "w") do |f|
+      f.set_encoding(Encoding::IBM437)
+      # A character whose codepoint differs between UTF-8 and IBM437
+      f.write "ƒ\n".freeze
+    end
+
+    File.binread(@filename).bytes.should == [159, 10]
+  end
+
   describe "on a pipe" do
     before :each do
       @r, @w = IO.pipe

spec/ruby/library/tempfile/initialize_spec.rb

@@ -38,4 +38,9 @@
     @tempfile.send(:initialize, ['shiftjis', 'yml'], encoding: 'SHIFT_JIS')
     @tempfile.external_encoding.should == Encoding::Shift_JIS
   end
+
+  it "does not try to modify the arguments" do
+    @tempfile.send(:initialize, ['frozen'.freeze, 'txt'.freeze], encoding: Encoding::IBM437)
+    @tempfile.external_encoding.should == Encoding::IBM437
+  end
 end

src/main/ruby/truffleruby/core/io.rb

@@ -2549,7 +2549,7 @@ def write(data)
        external_encoding != data.encoding &&
        external_encoding != Encoding::ASCII_8BIT
       unless data.ascii_only? && external_encoding.ascii_compatible?
-        data.encode!(external_encoding)
+        data = data.encode(external_encoding)
       end
     end
 

src/main/ruby/truffleruby/core/post.rb

@@ -60,11 +60,17 @@ def external_encoding
 STDERR.sync = true
 
 # Always flush standard streams on exit
-Truffle::KernelOperations.at_exit true do
+Truffle::KernelOperations.at_exit(true) do
   STDOUT.flush
+rescue Errno::EPIPE
+  # Could not write to STDOUT, the calling process closed the pipe
+  nil
 end
-Truffle::KernelOperations.at_exit true do
+Truffle::KernelOperations.at_exit(true) do
   STDERR.flush
+rescue Errno::EPIPE
+  # Could not write to STDERR, the calling process closed the pipe
+  nil
 end
 
 module Truffle