Backport #5700

fork {} segfaults during VM cleanup when run inside Fiber

Added by normalperson (Eric Wong) 6 months ago. Updated 3 months ago.

[ruby-core:41456]
Status:Closed Start date:12/03/2011
Priority:Normal Due date:
Assignee:- % Done:

100%

Category:-
Target version:-

Description

The issue is very easy to reproduce: Fiber.new do p Process.waitpid2(fork {}) end.resume Parent process successfully exits, child process segfaults while it is exiting. Backtrace is attached (gdb_bt.txt) MALLOC_CHECK_=3 with GNU libc malloc() implementation detects an attempt to free invalid pointer (attachment: malloc_check_3.txt) I can also reproduce this with 1.9.2-p290 and 1.9.3-p0 as well as latest trunk, so it has been around a while and a fix needs to be backported.

gdb_bt.txt - GDB backtrace (2.6 kB) normalperson (Eric Wong), 12/03/2011 10:37 am

malloc_check_3.txt - output with MALLOC_CHECK_=3 when run with GNU libc (6.1 kB) normalperson (Eric Wong), 12/03/2011 10:37 am

fiber_fork.rb - script to reproduce error (54 Bytes) normalperson (Eric Wong), 12/03/2011 10:37 am

bug5700.patch (2.1 kB) nagachika (Tomoyuki Chikanaga), 02/10/2012 12:30 pm

Associated revisions

Revision 34637
Added by naruse (Yui NARUSE) 3 months ago

merge revision(s) 34629,34630: * cont.c (rb_fiber_reset_root_local_storage): add a new function to restore rb_thread_t::local_storage. * cont.c (rb_obj_is_fiber): add a new function to tell finalizer to prevent fibers from destroy. * gc.c (rb_objspace_call_finalizer): don't sweep fibers at finalizing objspace. * internal.h (rb_fiber_reset_root_local_storage, rb_obj_is_fiber): add prototypes. * vm.c (ruby_vm_destruct): reset main thread's local_storage before free main thread. rb_thread_t::local_storage is replaced by fiber's local storage when forked from fiber, and it should be already freed when the fiber was destroyed. * test/ruby/test_fiber.rb (test_fork_from_fiber): add test for fork from fiber. when the fiber was destroyed. [ruby-core:41456] [Bug #5700]

History

Updated by normalperson (Eric Wong) 4 months ago

Eric Wong <normalperson@yhbt.net> wrote: > http://redmine.ruby-lang.org/issues/5700 Hello, I would like to see this fixed for the next 1.9.3 release. I'm pretty sure this is an easy fix for somebody already familiar with the Fiber/continuation code (unfortunately I am not familiar with it).

Updated by nagachika (Tomoyuki Chikanaga) 3 months ago

Hmm, it don't seems easy problem for me... Apparently I'm not so familiar with fiber. Anyway I've written a patch for this issue. I know it's somewhat ad-hoc, but it works. ko1 and nobu how do you think?

Updated by normalperson (Eric Wong) 3 months ago

Tomoyuki Chikanaga <nagachika00@gmail.com> wrote: > Anyway I've written a patch for this issue. I know it's somewhat ad-hoc, but it works. > ko1 and nobu how do you think? Thanks for looking into this. This fix works for me, too :>

Updated by nobu (Nobuyoshi Nakada) 3 months ago

One particular case is OK, two cases would be still OK, but three cases are not exceptional already. We'll need some standard way to tell if a given object can be discarded immediately or not, I guess. Anyway, it would be enough for the time being.

Updated by nagachika (Tomoyuki Chikanaga) 3 months ago

I agree. In fact we don't have to protect all Fibers but only a root Fiber of main Thread. But can I check-in previous patch for a temporary workaround? The next release of 1.9.3 is almost coming.

Updated by nagachika (Tomoyuki Chikanaga) 3 months ago

  • Tracker changed from Bug to Backport
  • Project changed from ruby-trunk to Backport93
  • Category deleted (core)
  • Target version deleted (2.0.0)
I've committed at r34629, r34630. Please backport them.

Updated by naruse (Yui NARUSE) 3 months ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100
This issue was solved with changeset r34637. Eric, thank you for reporting this issue. Your contribution to Ruby is greatly appreciated. May Ruby be with you. ---------- merge revision(s) 34629,34630: * cont.c (rb_fiber_reset_root_local_storage): add a new function to restore rb_thread_t::local_storage. * cont.c (rb_obj_is_fiber): add a new function to tell finalizer to prevent fibers from destroy. * gc.c (rb_objspace_call_finalizer): don't sweep fibers at finalizing objspace. * internal.h (rb_fiber_reset_root_local_storage, rb_obj_is_fiber): add prototypes. * vm.c (ruby_vm_destruct): reset main thread's local_storage before free main thread. rb_thread_t::local_storage is replaced by fiber's local storage when forked from fiber, and it should be already freed when the fiber was destroyed. * test/ruby/test_fiber.rb (test_fork_from_fiber): add test for fork from fiber. when the fiber was destroyed. [ruby-core:41456] [Bug #5700]

Also available in: Atom PDF