Project

General

Profile

Bug #14929

[PATCH] thread.c (do_select): fix leak on exception

Added by normalperson (Eric Wong) 11 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:88037]

Description

thread.c (do_select): fix leak on exception

When do_select is interrupted and raise happens from
RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
do not get freed, leading to a memory leak.  Wrap up all the
FD sets into a Ruby object to ensure the GC can release an
allocations made for rb_fdset_t.

This leak existed since Ruby 2.0.0 (r36430)

I found this bug because I was tracking down a problem
while working on timer-thread elimination.


Files

Associated revisions

Revision 8d0f5f1b
Added by normal 11 months ago

thread.c (do_select): fix leak on exception

When do_select is interrupted and raise happens from
RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
do not get freed, leading to a memory leak. Wrap up all the
FD sets into a Ruby object to ensure the GC can release an
allocations made for rb_fdset_t.

This leak existed since Ruby 2.0.0 (r36430)

[Bug #14929]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64007 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 64007
Added by normalperson (Eric Wong) 11 months ago

thread.c (do_select): fix leak on exception

When do_select is interrupted and raise happens from
RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
do not get freed, leading to a memory leak. Wrap up all the
FD sets into a Ruby object to ensure the GC can release an
allocations made for rb_fdset_t.

This leak existed since Ruby 2.0.0 (r36430)

[Bug #14929]

Revision 64007
Added by normal 11 months ago

thread.c (do_select): fix leak on exception

When do_select is interrupted and raise happens from
RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
do not get freed, leading to a memory leak. Wrap up all the
FD sets into a Ruby object to ensure the GC can release an
allocations made for rb_fdset_t.

This leak existed since Ruby 2.0.0 (r36430)

[Bug #14929]

Revision 64019
Added by ko1 (Koichi Sasada) 11 months ago

increase timeout seconds.

  • test/ruby/test_io.rb (test_select_leak): increase timeout seconds to pass this test on a high-load machine.

Revision 64020
Added by usa (Usaku NAKAMURA) 11 months ago

60 sec is not enough at all

Revision 95abe79e
Added by usa (Usaku NAKAMURA) 10 months ago

merge revision(s) 64007,64019,64020: [Backport #14929]

    thread.c (do_select): fix leak on exception

    When do_select is interrupted and raise happens from
    RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
    do not get freed, leading to a memory leak.  Wrap up all the
    FD sets into a Ruby object to ensure the GC can release an
    allocations made for rb_fdset_t.

    This leak existed since Ruby 2.0.0 (r36430)

    [Bug #14929]

    increase timeout seconds.
    * test/ruby/test_io.rb (test_select_leak): increase timeout seconds
      to pass this test on a high-load machine.


    60 sec is not enough at all

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@64561 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 64561
Added by usa (Usaku NAKAMURA) 10 months ago

merge revision(s) 64007,64019,64020: [Backport #14929]

thread.c (do_select): fix leak on exception

When do_select is interrupted and raise happens from
RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
do not get freed, leading to a memory leak.  Wrap up all the
FD sets into a Ruby object to ensure the GC can release an
allocations made for rb_fdset_t.

This leak existed since Ruby 2.0.0 (r36430)

[Bug #14929]

increase timeout seconds.
* test/ruby/test_io.rb (test_select_leak): increase timeout seconds
  to pass this test on a high-load machine.


60 sec is not enough at all

Revision c91b7154
Added by nagachika (Tomoyuki Chikanaga) 10 months ago

merge revision(s) 64007,64019,64020: [Backport #14929]

    thread.c (do_select): fix leak on exception

    When do_select is interrupted and raise happens from
    RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
    do not get freed, leading to a memory leak.  Wrap up all the
    FD sets into a Ruby object to ensure the GC can release an
    allocations made for rb_fdset_t.

    This leak existed since Ruby 2.0.0 (r36430)

    [Bug #14929]

    increase timeout seconds.
    * test/ruby/test_io.rb (test_select_leak): increase timeout seconds
      to pass this test on a high-load machine.


    60 sec is not enough at all

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@64605 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 64605
Added by nagachika (Tomoyuki Chikanaga) 10 months ago

merge revision(s) 64007,64019,64020: [Backport #14929]

thread.c (do_select): fix leak on exception

When do_select is interrupted and raise happens from
RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
do not get freed, leading to a memory leak.  Wrap up all the
FD sets into a Ruby object to ensure the GC can release an
allocations made for rb_fdset_t.

This leak existed since Ruby 2.0.0 (r36430)

[Bug #14929]

increase timeout seconds.
* test/ruby/test_io.rb (test_select_leak): increase timeout seconds
  to pass this test on a high-load machine.


60 sec is not enough at all

History

#1

Updated by normalperson (Eric Wong) 11 months ago

  • Status changed from Open to Closed

Applied in changeset trunk|r64007.


thread.c (do_select): fix leak on exception

When do_select is interrupted and raise happens from
RUBY_VM_CHECK_INTS_BLOCKING, the original FD sets we copied
do not get freed, leading to a memory leak. Wrap up all the
FD sets into a Ruby object to ensure the GC can release an
allocations made for rb_fdset_t.

This leak existed since Ruby 2.0.0 (r36430)

[Bug #14929]

Updated by usa (Usaku NAKAMURA) 10 months ago

  • Backport changed from 2.3: REQUIRED, 2.4: REQUIRED, 2.5: REQUIRED to 2.3: REQUIRED, 2.4: DONE, 2.5: REQUIRED

ruby_2_4 r64561 merged revision(s) 64007,64019,64020.

Updated by nagachika (Tomoyuki Chikanaga) 10 months ago

  • Backport changed from 2.3: REQUIRED, 2.4: DONE, 2.5: REQUIRED to 2.3: REQUIRED, 2.4: DONE, 2.5: DONE

ruby_2_5 r64605 merged revision(s) 64007,64019,64020.

Updated by jaruga (Jun Aruga) 7 months ago

Could you backport below commits for test/ruby/test_io.rb#test_select_leak from ruby_2_5 or trunk to ruby_2_4 branch?

ruby_2_5 branch

$ git diff ruby_2_4..ruby_2_5 test/ruby/test_io.rb
...
   def test_select_leak
-    assert_no_memory_leak([], <<-"end;", <<-"end;", rss: true, timeout: 60)
+    assert_no_memory_leak([], <<-"end;", <<-"end;", rss: true, timeout: 240)
       r, w = IO.pipe
       rset = [r]
       wset = [w]
@@ -3562,6 +3755,7 @@ def test_select_leak
         Thread.pass until th.stop?
         th.kill
         th.join
+        GC.start
       end
     end;
   end

trunk branch

$ git diff ruby_2_4..trunk test/ruby/test_io.rb
...
   def test_select_leak
-    assert_no_memory_leak([], <<-"end;", <<-"end;", rss: true, timeout: 60)
+    # avoid malloc arena explosion from glibc and jemalloc:
...
       end
+      th.kill
+      th.join
     end;
   end

I faced this issue when building ruby-2.4 on on Fedora multi archs environment.

I think that this also can fix on rubyci ruby_2.4 test.
https://rubyci.org/logs/rubyci.s3.amazonaws.com/freebsd11zfs/ruby-2.4/log/20181112T102145Z.fail.html.gz

Thank you!

Also available in: Atom PDF