Skip to content
This repository

Segfault when forked Queue is being finalized by GC #1

Closed
netshade opened this Issue March 26, 2012 · 2 comments

3 participants

Chris Zelenak Maksym Melnychok
Chris Zelenak

The following code will cause an error when executed on the command line w/ MBARI api enabled:

ruby  -rthread -e 'q = Queue.new; Thread.new { q.pop }; pid = fork;  if pid.nil?; q = nil; GC.start; else; Process.wait(pid); end'

The error looks like

-e:1:in `start': wrong argument type Object (expected Thread) (TypeError)

In an active Ruby script / webapp, a segfault is likely to occur instead. The segfault occurs in rb_thread_check() in the forked child process, which is checking the validity of a pointer to a thread from the parent process before it kills the thread. (For full background, rb_thread_check() is being called by rb_thread_kill(), which is in turn being called by the Queue's object finalizer ext/thread/thread.c#free_queue() as the GC evicts the object) In the one liner example above, the pointer ends up pointing to another RVALUE, while in more complex cases the pointer's value is unknown.

Tested on branch v1_8_7_352-mbari, w/

./configure --enable-mbari-api CFLAGS="-O2 -fno-stack-protector"

and

./configure --with-wipe-sites=0x0000 --enable-mbari-api CFLAGS="-O2 -fno-stack-protector"

to ensure that stack wiping was not creating the problem.

Iterating through the commit list on v1_8_7_352-mbari has shown that the bug first appears after commit 32e862 . Testing on stock 1.8.7 does not show the bug, nor does testing on the 1.9 series.

Maksym Melnychok

any chance this will get fixed?

Owner

Sorry, I just saw this issue for the first time a couple days ago.
A fix has been pushed to github. Let me know how they work for you.
Note:
The problem is not really specific to ruby with the MBARI patch set.
See below:

The fastthread lib kills all threads waiting on a queue when that queue
finalized. If this happens in the child process of a fork, those threads
will likely have already been destroyed and may be slated for
finalization themselves. Trying to run them (even for kill) leads to
a exception out of in rb_thread_check()

This patch makes fastthread check whether the threads are intact
before trying to kill them.  Netshade provided the following test case:

ruby  -rthread -e '
q = Queue.new; Thread.new { q.pop }; pid = fork;
if pid.nil?; q = nil; GC.start; else; Process.wait(pid); end'

The test case fails more often when the MBARI patches are applied because
they make GC work better -- so, it is more likely that the threads
will be immediately slated for finalization by the first pass after fork.

This patch should also be applied to the MRI mainline.
brentr closed this December 01, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.