Bug #9205

Assertion failed: heap_pages_deferred_final == 0

Added by Heesob Park over 1 year ago. Updated about 1 year ago.

[ruby-core:58833]
Status:Closed
Priority:Normal
Assignee:-
ruby -v:ruby 2.1.0dev (2013-12-02 trunk 43958) [x64-mswin64_120] Backport:1.9.3: DONE, 2.0.0: DONE

Description

If I run the following code:

raise_proc = proc do |id|
  p "Finalizer one on #{id}" 
end
10000.times do
  ObjectSpace.define_finalizer(Object.new, raise_proc)
end

I can see an assertion failure.

"Finalizer one on 24535120"
Assertion failed: heap_pages_deferred_final == 0, file gc.c, line 2111

Same error with ruby 2.0.0p353 (2013-11-22) [i386-mingw32]

fix_assertion_fail.patch Magnifier (981 Bytes) Heesob Park, 12/04/2013 12:39 PM


Related issues

Related to Backport193 - Backport #9463: SEGV when calling GC.start in a finalizer Closed 01/30/2014

Associated revisions

Revision 43994
Added by Nobuyoshi Nakada over 1 year ago

gc.c: flush all deferred finalizers

  • gc.c (finalize_deferred): flush all deferred finalizers while other finalizers can get ready to run newly by lazy sweep. [Bug #9205]

Revision 43994
Added by Nobuyoshi Nakada over 1 year ago

gc.c: flush all deferred finalizers

  • gc.c (finalize_deferred): flush all deferred finalizers while other finalizers can get ready to run newly by lazy sweep. [Bug #9205]

History

#1 Updated by Heesob Park over 1 year ago

I found that make_deferred function of gc.c modified the value of heap_pages_deferred_final to nonzero.
I made a draft patch and I am not sure this patch is the best.
Anyway, it fixed the assertion and passed test-all.

#2 Updated by Nobuyoshi Nakada over 1 year ago

It doesn't seem nice to just ignore finalizers while running other finalizers.

diff --git c/gc.c i/gc.c
index ddf607c..0d18031 100644
--- c/gc.c
+++ i/gc.c
@@ -2055,10 +2055,10 @@ finalize_list(rb_objspace_t *objspace, RVALUE *p)
 static void
 finalize_deferred(rb_objspace_t *objspace)
 {
-    RVALUE *p = heap_pages_deferred_final;
-    heap_pages_deferred_final = 0;
+    RVALUE *p;

-    if (p) {
+    while ((p = heap_pages_deferred_final) != 0) {
+   heap_pages_deferred_final = 0;
    finalize_list(objspace, p);
     }
 }
diff --git c/test/ruby/test_gc.rb i/test/ruby/test_gc.rb
index 034a330..2b1e6a5 100644
--- c/test/ruby/test_gc.rb
+++ i/test/ruby/test_gc.rb
@@ -238,4 +238,19 @@ class TestGc < Test::Unit::TestCase
     assert_not_nil GC::INTERNAL_CONSTANTS[:HEAP_OBJ_LIMIT]
     assert_not_nil GC::INTERNAL_CONSTANTS[:RVALUE_SIZE]
   end
+
+  def test_raise_in_finalizer
+    bug9205 = ' [Bug #9205]'
+    100.times do
+      assert_ruby_status([], <<-'end;', bug9205)
+        STDOUT.sync = true
+        raise_proc = proc do |id|
+          p "Finalizer one on #{id}"
+        end
+        10000.times do
+          ObjectSpace.define_finalizer(Object.new, raise_proc)
+        end
+      end;
+    end
+  end
 end

#3 Updated by Heesob Park over 1 year ago

You are right.
Why not commit your patch?

#4 Updated by Nobuyoshi Nakada over 1 year ago

I've misread the code, like as raise_proc raises an exception and that triggers the failure.
But the proc raises nothing, then what would trigger it?

#5 Updated by Heesob Park over 1 year ago

My test code is different from Bug #9168.
I guess the name raise_proc made confusion.
It is just repeated finalizer test without raising an exception.

#6 Updated by Nobuyoshi Nakada over 1 year ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r43994.
Heesob, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


gc.c: flush all deferred finalizers

  • gc.c (finalize_deferred): flush all deferred finalizers while other finalizers can get ready to run newly by lazy sweep. [Bug #9205]

#7 Updated by Tomoyuki Chikanaga over 1 year ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: UNKNOWN, 2.0.0: REQUIRED

#8 Updated by Tomoyuki Chikanaga over 1 year ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: REQUIRED to 1.9.3: UNKNOWN, 2.0.0: DONE

r43994 was backported to ruby_2_0_0 at r44319.

#9 Updated by Tomoyuki Chikanaga over 1 year ago

... and r44000 was also backported to ruby_2_0_0 at r44322.

#10 Updated by Usaku NAKAMURA about 1 year ago

  • Related to Backport #9463: SEGV when calling GC.start in a finalizer added

#11 Updated by Usaku NAKAMURA about 1 year ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: DONE to 1.9.3: DONE, 2.0.0: DONE

backported into ruby_1_9_3 at r44764.

Also available in: Atom PDF