Project

General

Profile

Bug #14929

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

Added by normalperson (Eric Wong) about 1 year ago. Updated 10 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 about 1 year 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) about 1 year 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 about 1 year 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) about 1 year 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) about 1 year ago

60 sec is not enough at all

Revision 95abe79e
Added by usa (Usaku NAKAMURA) about 1 year 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) about 1 year 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) about 1 year 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) about 1 year 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) about 1 year 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) about 1 year 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) about 1 year 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) 10 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