[GR-32325] Kernel#exit!, Fiber kill and internal errors should not run code in ensure

PullRequest: truffleruby/3638

Author: eregon

CHANGELOG.md

@@ -75,6 +75,7 @@ Compatibility:
 * Fix `Array` methods `select!` and `keep_if` and handle a case when exception is raised in a passed block properly (@andrykonchin).
 * Fix `Enumerable` methods `each_cons` and `each_slice` to return receiver (#2733, @horakivo)
 * `Module` methods `#private`, `#public`, `#protected`, `#module_function` now returns their arguments like in CRuby 3.1 (#2733, @horakivo)
+* `Kernel#exit!`, killing Fibers and internal errors do not run code in `ensure` clauses anymore, the same as CRuby (@eregon).
 
 Performance:
 

spec/ruby/core/exception/fixtures/thread_fiber_ensure.rb

@@ -0,0 +1,22 @@
+ready = false
+t = Thread.new do
+  f = Fiber.new do
+    begin
+      Fiber.yield
+    ensure
+      STDERR.puts "suspended fiber ensure"
+    end
+  end
+  f.resume
+
+  begin
+    ready = true
+    sleep
+  ensure
+    STDERR.puts "current fiber ensure"
+  end
+end
+
+Thread.pass until ready && t.stop?
+
+# let the program end, it's the same as #exit or an exception for this behavior

spec/ruby/core/exception/fixtures/thread_fiber_ensure_non_root_fiber.rb

@@ -0,0 +1,25 @@
+ready = false
+t = Thread.new do
+  f = Fiber.new do
+    begin
+      Fiber.yield
+    ensure
+      STDERR.puts "suspended fiber ensure"
+    end
+  end
+  f.resume
+
+  f2 = Fiber.new do
+    begin
+      ready = true
+      sleep
+    ensure
+      STDERR.puts "current fiber ensure"
+    end
+  end
+  f2.resume
+end
+
+Thread.pass until ready && t.stop?
+
+# let the program end, it's the same as #exit or an exception for this behavior

spec/ruby/core/exception/top_level_spec.rb

@@ -42,4 +42,16 @@ def raise_wrapped
       EOS
     end
   end
+
+  describe "kills all threads and fibers, ensure clauses are only run for threads current fibers, not for suspended fibers" do
+    it "with ensure on the root fiber" do
+      file = fixture(__FILE__, "thread_fiber_ensure.rb")
+      ruby_exe(file, args: "2>&1", exit_status: 0).should == "current fiber ensure\n"
+    end
+
+    it "with ensure on non-root fiber" do
+      file = fixture(__FILE__, "thread_fiber_ensure_non_root_fiber.rb")
+      ruby_exe(file, args: "2>&1", exit_status: 0).should == "current fiber ensure\n"
+    end
+  end
 end

spec/ruby/core/kernel/exit_spec.rb

@@ -10,6 +10,10 @@
   it_behaves_like :process_exit, :exit, KernelSpecs::Method.new
 end
 
+describe "Kernel.exit" do
+  it_behaves_like :process_exit, :exit, Kernel
+end
+
 describe "Kernel#exit!" do
   it "is a private method" do
     Kernel.should have_private_instance_method(:exit!)
@@ -18,10 +22,6 @@
   it_behaves_like :process_exit!, :exit!, "self"
 end
 
-describe "Kernel.exit" do
-  it_behaves_like :process_exit, :exit, Kernel
-end
-
 describe "Kernel.exit!" do
-  it_behaves_like :process_exit!, :exit!, Kernel
+  it_behaves_like :process_exit!, :exit!, "Kernel"
 end

spec/ruby/core/process/exit_spec.rb

@@ -6,5 +6,5 @@
 end
 
 describe "Process.exit!" do
-  it_behaves_like :process_exit!, :exit!, Process
+  it_behaves_like :process_exit!, :exit!, "Process"
 end

spec/ruby/core/thread/kill_spec.rb

@@ -9,10 +9,6 @@
     it_behaves_like :thread_exit, :kill
   end
 
-  describe "Thread#kill!" do
-    it "needs to be reviewed for spec completeness"
-  end
-
   describe "Thread.kill" do
     it "causes the given thread to exit" do
       thread = Thread.new { sleep }

spec/ruby/core/thread/shared/exit.rb

@@ -113,6 +113,25 @@
     ScratchPad.recorded.should == nil
   end
 
+  it "kills other fibers of that thread without running their ensure clauses" do
+    t = Thread.new do
+      f = Fiber.new do
+        ScratchPad.record :fiber_resumed
+        begin
+          Fiber.yield
+        ensure
+          ScratchPad.record :fiber_ensure
+        end
+      end
+      f.resume
+      sleep
+    end
+    Thread.pass until t.stop?
+    t.send(@method)
+    t.join
+    ScratchPad.recorded.should == :fiber_resumed
+  end
+
   # This spec is a mess. It fails randomly, it hangs on MRI, it needs to be removed
   quarantine! do
   it "killing dying running does nothing" do

spec/ruby/shared/process/exit.rb

@@ -104,6 +104,12 @@
     $?.exitstatus.should == 21
   end
 
+  it "skips ensure clauses" do
+    out = ruby_exe("begin; STDERR.puts 'before'; #{@object}.send(:exit!, 21); ensure; STDERR.puts 'ensure'; end", args: '2>&1', exit_status: 21)
+    out.should == "before\n"
+    $?.exitstatus.should == 21
+  end
+
   it "overrides the original exception and exit status when called from #at_exit" do
     code = <<-RUBY
     at_exit do

spec/tags/core/exception/top_level_tags.txt

@@ -1,3 +1,6 @@
 slow:An Exception reaching the top level is printed on STDERR
 slow:An Exception reaching the top level with a custom backtrace is printed on STDERR
 slow:An Exception reaching the top level the Exception#cause is printed to STDERR with backtraces
+slow:An Exception reaching the top level kills all threads and fibers, ensure clauses are only run for threads current fibers, not for suspended fibers with ensure on the root fiber
+slow:An Exception reaching the top level kills all threads and fibers, ensure clauses are only run for threads current fibers, not for suspended fibers with ensure on non-root fiber
+fails(GR-44211):An Exception reaching the top level kills all threads and fibers, ensure clauses are only run for threads current fibers, not for suspended fibers with ensure on non-root fiber