Skip to content

Commit 784b3dd

Browse files
tenderlovetenderlove
authored
Merge pull request #40 from github/fix-for-14634
Fix for 14634
2 parents 4501602 + d2986e0 commit 784b3dd

File tree

7 files changed

+160
-12
lines changed

7 files changed

+160
-12
lines changed

‎test/ruby/test_autoload.rb‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,32 @@ class AutoloadTest
285285
end
286286
end
287287

288+
def test_autoload_fork
289+
EnvUtil.default_warning do
290+
Tempfile.create(['autoload', '.rb']) {|file|
291+
file.puts 'sleep 0.3; class AutoloadTest; end'
292+
file.close
293+
add_autoload(file.path)
294+
begin
295+
thrs = []
296+
3.times do
297+
thrs << Thread.new { AutoloadTest; nil }
298+
thrs << Thread.new { fork { AutoloadTest } }
299+
end
300+
thrs.each(&:join)
301+
thrs.each do |th|
302+
pid = th.value or next
303+
_, status = Process.waitpid2(pid)
304+
assert_predicate status, :success?
305+
end
306+
ensure
307+
remove_autoload_constant
308+
assert_nil $!, '[ruby-core:86410] [Bug #14634]'
309+
end
310+
}
311+
end
312+
end if Process.respond_to?(:fork)
313+
288314
def add_autoload(path)
289315
(@autoload_paths ||= []) << path
290316
::Object.class_eval {autoload(:AutoloadTest, path)}

‎test/thread/test_cv.rb‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,23 @@ def test_dump
217217
Marshal.dump(condvar)
218218
end
219219
end
220+
221+
def test_condvar_fork
222+
mutex = Mutex.new
223+
condvar = ConditionVariable.new
224+
thrs = (1..10).map do
225+
Thread.new { mutex.synchronize { condvar.wait(mutex) } }
226+
end
227+
thrs.each { 3.times { Thread.pass } }
228+
pid = fork do
229+
mutex.synchronize { condvar.broadcast }
230+
exit!(0)
231+
end
232+
_, s = Process.waitpid2(pid)
233+
assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]'
234+
until thrs.empty?
235+
mutex.synchronize { condvar.broadcast }
236+
thrs.delete_if { |t| t.join(0.01) }
237+
end
238+
end if Process.respond_to?(:fork)
220239
end

‎test/thread/test_queue.rb‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,4 +565,52 @@ def test_queue_with_trap
565565
puts 'exit'
566566
INPUT
567567
end
568+
569+
def test_fork_while_queue_waiting
570+
q = Queue.new
571+
sq = SizedQueue.new(1)
572+
thq = Thread.new { q.pop }
573+
thsq = Thread.new { sq.pop }
574+
Thread.pass until thq.stop? && thsq.stop?
575+
576+
pid = fork do
577+
exit!(1) if q.num_waiting != 0
578+
exit!(2) if sq.num_waiting != 0
579+
exit!(6) unless q.empty?
580+
exit!(7) unless sq.empty?
581+
q.push :child_q
582+
sq.push :child_sq
583+
exit!(3) if q.pop != :child_q
584+
exit!(4) if sq.pop != :child_sq
585+
exit!(0)
586+
end
587+
_, s = Process.waitpid2(pid)
588+
assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]'
589+
590+
q.push :thq
591+
sq.push :thsq
592+
assert_equal :thq, thq.value
593+
assert_equal :thsq, thsq.value
594+
595+
sq.push(1)
596+
th = Thread.new { q.pop; sq.pop }
597+
thsq = Thread.new { sq.push(2) }
598+
Thread.pass until th.stop? && thsq.stop?
599+
pid = fork do
600+
exit!(1) if q.num_waiting != 0
601+
exit!(2) if sq.num_waiting != 0
602+
exit!(3) unless q.empty?
603+
exit!(4) if sq.empty?
604+
exit!(5) if sq.pop != 1
605+
exit!(0)
606+
end
607+
_, s = Process.waitpid2(pid)
608+
assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]'
609+
610+
assert_predicate thsq, :stop?
611+
assert_equal 1, sq.pop
612+
assert_same sq, thsq.value
613+
q.push('restart th')
614+
assert_equal 2, th.value
615+
end if Process.respond_to?(:fork)
568616
end

‎thread.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4188,6 +4188,8 @@ rb_thread_atfork_internal(rb_thread_t *th, void (*atfork)(rb_thread_t *, const r
41884188
}
41894189
rb_vm_living_threads_init(vm);
41904190
rb_vm_living_threads_insert(vm, th);
4191+
vm->fork_gen++;
4192+
41914193
vm->sleeper = 0;
41924194
clear_coverage();
41934195
}

‎thread_sync.c‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@
44
static VALUE rb_cMutex, rb_cQueue, rb_cSizedQueue, rb_cConditionVariable;
55
static VALUE rb_eClosedQueueError;
66

7+
/*
8+
* keep these globally so we can walk and reinitialize them at fork
9+
* in the child process
10+
*/
11+
static LIST_HEAD(szqueue_list);
12+
static LIST_HEAD(queue_list);
13+
static LIST_HEAD(condvar_list);
14+
715
/* sync_waiter is always on-stack */
816
struct sync_waiter {
917
rb_thread_t *th;
@@ -537,6 +545,7 @@ void rb_mutex_allow_trap(VALUE self, int val)
537545
#define queue_waitq(q) UNALIGNED_MEMBER_PTR(q, waitq)
538546
PACKED_STRUCT_UNALIGNED(struct rb_queue {
539547
struct list_head waitq;
548+
rb_serial_t fork_gen;
540549
const VALUE que;
541550
int num_waiting;
542551
});
@@ -582,12 +591,29 @@ queue_alloc(VALUE klass)
582591
return obj;
583592
}
584593

594+
static int
595+
queue_fork_check(struct rb_queue *q)
596+
{
597+
rb_serial_t fork_gen = GET_VM()->fork_gen;
598+
599+
if (q->fork_gen == fork_gen) {
600+
return 0;
601+
}
602+
/* forked children can't reach into parent thread stacks */
603+
q->fork_gen = fork_gen;
604+
list_head_init(queue_waitq(q));
605+
q->num_waiting = 0;
606+
return 1;
607+
}
608+
585609
static struct rb_queue *
586610
queue_ptr(VALUE obj)
587611
{
588612
struct rb_queue *q;
589613

590614
TypedData_Get_Struct(obj, struct rb_queue, &queue_data_type, q);
615+
queue_fork_check(q);
616+
591617
return q;
592618
}
593619

@@ -630,6 +656,11 @@ szqueue_ptr(VALUE obj)
630656
struct rb_szqueue *sq;
631657

632658
TypedData_Get_Struct(obj, struct rb_szqueue, &szqueue_data_type, sq);
659+
if (queue_fork_check(&sq->q)) {
660+
list_head_init(szqueue_pushq(sq));
661+
sq->num_waiting_push = 0;
662+
}
663+
633664
return sq;
634665
}
635666

@@ -866,7 +897,7 @@ queue_do_pop(VALUE self, struct rb_queue *q, int should_block)
866897
list_add_tail(&qw.as.q->waitq, &qw.w.node);
867898
qw.as.q->num_waiting++;
868899

869-
rb_ensure(queue_sleep, Qfalse, queue_sleep_done, (VALUE)&qw);
900+
rb_ensure(queue_sleep, self, queue_sleep_done, (VALUE)&qw);
870901
}
871902
}
872903

@@ -1108,7 +1139,7 @@ rb_szqueue_push(int argc, VALUE *argv, VALUE self)
11081139
list_add_tail(pushq, &qw.w.node);
11091140
sq->num_waiting_push++;
11101141

1111-
rb_ensure(queue_sleep, Qfalse, szqueue_sleep_done, (VALUE)&qw);
1142+
rb_ensure(queue_sleep, self, szqueue_sleep_done, (VALUE)&qw);
11121143
}
11131144
}
11141145

@@ -1209,9 +1240,9 @@ rb_szqueue_empty_p(VALUE self)
12091240

12101241

12111242
/* ConditionalVariable */
1212-
/* TODO: maybe this can be IMEMO */
12131243
struct rb_condvar {
12141244
struct list_head waitq;
1245+
rb_serial_t fork_gen;
12151246
};
12161247

12171248
/*
@@ -1258,9 +1289,16 @@ static struct rb_condvar *
12581289
condvar_ptr(VALUE self)
12591290
{
12601291
struct rb_condvar *cv;
1292+
rb_serial_t fork_gen = GET_VM()->fork_gen;
12611293

12621294
TypedData_Get_Struct(self, struct rb_condvar, &cv_data_type, cv);
12631295

1296+
/* forked children can't reach into parent thread stacks */
1297+
if (cv->fork_gen != fork_gen) {
1298+
cv->fork_gen = fork_gen;
1299+
list_head_init(&cv->waitq);
1300+
}
1301+
12641302
return cv;
12651303
}
12661304

‎variable.c‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "ccan/list/list.h"
2121
#include "id_table.h"
2222
#include "debug_counter.h"
23+
#include "vm_core.h"
2324

2425
struct rb_id_table *rb_global_tbl;
2526
static ID autoload, classpath, tmp_classpath, classid;
@@ -1859,6 +1860,7 @@ struct autoload_data_i {
18591860
rb_const_flag_t flag;
18601861
VALUE value;
18611862
struct autoload_state *state; /* points to on-stack struct */
1863+
rb_serial_t fork_gen;
18621864
};
18631865

18641866
static void
@@ -1881,8 +1883,18 @@ static const rb_data_type_t autoload_data_i_type = {
18811883
0, 0, RUBY_TYPED_FREE_IMMEDIATELY
18821884
};
18831885

1884-
#define check_autoload_data(av) \
1885-
(struct autoload_data_i *)rb_check_typeddata((av), &autoload_data_i_type)
1886+
static struct autoload_data_i *
1887+
get_autoload_data(VALUE av)
1888+
{
1889+
struct autoload_data_i *ele = rb_check_typeddata(av, &autoload_data_i_type);
1890+
1891+
/* do not reach across stack for ->state after forking: */
1892+
if (ele && ele->state && ele->fork_gen != GET_VM()->fork_gen) {
1893+
ele->state = 0;
1894+
ele->fork_gen = 0;
1895+
}
1896+
return ele;
1897+
}
18861898

18871899
RUBY_FUNC_EXPORTED void
18881900
rb_autoload(VALUE mod, ID id, const char *file)
@@ -1982,7 +1994,7 @@ check_autoload_required(VALUE mod, ID id, const char **loadingpath)
19821994
const char *loading;
19831995
int safe;
19841996

1985-
if (!(load = autoload_data(mod, id)) || !(ele = check_autoload_data(load))) {
1997+
if (!(load = autoload_data(mod, id)) || !(ele = get_autoload_data(load))) {
19861998
return 0;
19871999
}
19882000
file = ele->feature;
@@ -2020,7 +2032,7 @@ rb_autoloading_value(VALUE mod, ID id, VALUE* value, rb_const_flag_t *flag)
20202032
VALUE load;
20212033
struct autoload_data_i *ele;
20222034

2023-
if (!(load = autoload_data(mod, id)) || !(ele = check_autoload_data(load))) {
2035+
if (!(load = autoload_data(mod, id)) || !(ele = get_autoload_data(load))) {
20242036
return 0;
20252037
}
20262038
if (ele->state && ele->state->thread == rb_thread_current()) {
@@ -2087,8 +2099,9 @@ autoload_reset(VALUE arg)
20872099
int need_wakeups = 0;
20882100

20892101
if (state->ele->state == state) {
2090-
need_wakeups = 1;
2091-
state->ele->state = 0;
2102+
need_wakeups = 1;
2103+
state->ele->state = 0;
2104+
state->ele->fork_gen = 0;
20922105
}
20932106

20942107
/* At the last, move a value defined in autoload to constant table */
@@ -2170,7 +2183,7 @@ rb_autoload_load(VALUE mod, ID id)
21702183
if (src && loading && strcmp(src, loading) == 0) return Qfalse;
21712184

21722185
/* set ele->state for a marker of autoloading thread */
2173-
if (!(ele = check_autoload_data(load))) {
2186+
if (!(ele = get_autoload_data(load))) {
21742187
return Qfalse;
21752188
}
21762189

@@ -2180,6 +2193,7 @@ rb_autoload_load(VALUE mod, ID id)
21802193
state.thread = rb_thread_current();
21812194
if (!ele->state) {
21822195
ele->state = &state;
2196+
ele->fork_gen = GET_VM()->fork_gen;
21832197

21842198
/*
21852199
* autoload_reset will wake up any threads added to this
@@ -2217,7 +2231,7 @@ rb_autoload_p(VALUE mod, ID id)
22172231
}
22182232
load = check_autoload_required(mod, id, 0);
22192233
if (!load) return Qnil;
2220-
return (ele = check_autoload_data(load)) ? ele->feature : Qnil;
2234+
return (ele = get_autoload_data(load)) ? ele->feature : Qnil;
22212235
}
22222236

22232237
void
@@ -2646,7 +2660,7 @@ current_autoload_data(VALUE mod, ID id)
26462660
struct autoload_data_i *ele;
26472661
VALUE load = autoload_data(mod, id);
26482662
if (!load) return 0;
2649-
ele = check_autoload_data(load);
2663+
ele = get_autoload_data(load);
26502664
if (!ele) return 0;
26512665
/* for autoloading thread, keep the defined value to autoloading storage */
26522666
if (ele->state && (ele->state->thread == rb_thread_current())) {

‎vm_core.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ typedef struct rb_vm_struct {
507507
struct rb_thread_struct *main_thread;
508508
struct rb_thread_struct *running_thread;
509509

510+
rb_serial_t fork_gen;
510511
struct list_head waiting_fds; /* <=> struct waiting_fd */
511512
struct list_head living_threads;
512513
size_t living_thread_num;

0 commit comments

Comments
 (0)