merge revision(s) 62934,63210,63215,63309: [Backport #14634]
thread_sync.c: avoid reaching across stacks of dead threads rb_ensure is insufficient cleanup for fork and we must reinitialize all waitqueues in the child process. Unfortunately this increases the footprint of ConditionVariable, Queue and SizedQueue by 8 bytes on 32-bit (16 bytes on 64-bit). [ruby-core:86316] [Bug #14634] variable.c: fix thread + fork errors in autoload This is fairly non-intrusive bugfix to prevent children from trying to reach into thread stacks of the parent. I will probably reuse this idea and redo r62934, too (same bug). * vm_core.h (typedef struct rb_vm_struct): add fork_gen counter * thread.c (rb_thread_atfork_internal): increment fork_gen * variable.c (struct autoload_data_i): store fork_gen * variable.c (check_autoload_data): remove (replaced with get_...) * variable.c (get_autoload_data): check fork_gen when retrieving * variable.c (check_autoload_required): use get_autoload_data * variable.c (rb_autoloading_value): ditto * variable.c (rb_autoload_p): ditto * variable.c (current_autoload_data): ditto * variable.c (autoload_reset): reset fork_gen, adjust indent * variable.c (rb_autoload_load): set fork_gen when setting state * test/ruby/test_autoload.rb (test_autoload_fork): new test [ruby-core:86410] [Bug #14634] thread_sync: redo r62934 to use fork_gen Instead of maintaining linked-lists to store all rb_queue/rb_szqueue/rb_condvar structs; store only a fork_gen serial number to simplify management of these items. This reduces initialization costs and avoids the up-front cost of resetting all Queue/SizedQueue/ConditionVariable objects at fork while saving 8 bytes per-structure on 64-bit. There are no savings on 32-bit. * thread.c (rb_thread_atfork_internal): remove rb_thread_sync_reset_all call * thread_sync.c (rb_thread_sync_reset_all): remove * thread_sync.c (queue_live): remove * thread_sync.c (queue_free): remove * thread_sync.c (struct rb_queue): s/live/fork_gen/ * thread_sync.c (queue_data_type): use default free * thread_sync.c (queue_alloc): remove list_add * thread_sync.c (queue_fork_check): new function * thread_sync.c (queue_ptr): call queue_fork_check * thread_sync.c (szqueue_free): remove * thread_sync.c (szqueue_data_type): use default free * thread_sync.c (szqueue_alloc): remove list_add * thread_sync.c (szqueue_ptr): check fork_gen via queue_fork_check * thread_sync.c (struct rb_condvar): s/live/fork_gen/ * thread_sync.c (condvar_free): remove * thread_sync.c (cv_data_type): use default free * thread_sync.c (condvar_ptr): check fork_gen * thread_sync.c (condvar_alloc): remove list_add [ruby-core:86316] [Bug #14634] thread_sync.c (condvar_ptr): reset fork_gen after forking Otherwise the condition variable waiter list will always be empty, which is wrong :x [Bug #14725] [Bug #14634] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@66912 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
This commit is contained in:
parent
e42dcdb515
commit
63d9ab33fe
|
@ -285,6 +285,32 @@ p Foo::Bar
|
|||
end
|
||||
end
|
||||
|
||||
def test_autoload_fork
|
||||
EnvUtil.default_warning do
|
||||
Tempfile.create(['autoload', '.rb']) {|file|
|
||||
file.puts 'sleep 0.3; class AutoloadTest; end'
|
||||
file.close
|
||||
add_autoload(file.path)
|
||||
begin
|
||||
thrs = []
|
||||
3.times do
|
||||
thrs << Thread.new { AutoloadTest; nil }
|
||||
thrs << Thread.new { fork { AutoloadTest } }
|
||||
end
|
||||
thrs.each(&:join)
|
||||
thrs.each do |th|
|
||||
pid = th.value or next
|
||||
_, status = Process.waitpid2(pid)
|
||||
assert_predicate status, :success?
|
||||
end
|
||||
ensure
|
||||
remove_autoload_constant
|
||||
assert_nil $!, '[ruby-core:86410] [Bug #14634]'
|
||||
end
|
||||
}
|
||||
end
|
||||
end if Process.respond_to?(:fork)
|
||||
|
||||
def add_autoload(path)
|
||||
(@autoload_paths ||= []) << path
|
||||
::Object.class_eval {autoload(:AutoloadTest, path)}
|
||||
|
|
|
@ -217,4 +217,23 @@ INPUT
|
|||
Marshal.dump(condvar)
|
||||
end
|
||||
end
|
||||
|
||||
def test_condvar_fork
|
||||
mutex = Mutex.new
|
||||
condvar = ConditionVariable.new
|
||||
thrs = (1..10).map do
|
||||
Thread.new { mutex.synchronize { condvar.wait(mutex) } }
|
||||
end
|
||||
thrs.each { 3.times { Thread.pass } }
|
||||
pid = fork do
|
||||
mutex.synchronize { condvar.broadcast }
|
||||
exit!(0)
|
||||
end
|
||||
_, s = Process.waitpid2(pid)
|
||||
assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]'
|
||||
until thrs.empty?
|
||||
mutex.synchronize { condvar.broadcast }
|
||||
thrs.delete_if { |t| t.join(0.01) }
|
||||
end
|
||||
end if Process.respond_to?(:fork)
|
||||
end
|
||||
|
|
|
@ -565,4 +565,52 @@ class TestQueue < Test::Unit::TestCase
|
|||
puts 'exit'
|
||||
INPUT
|
||||
end
|
||||
|
||||
def test_fork_while_queue_waiting
|
||||
q = Queue.new
|
||||
sq = SizedQueue.new(1)
|
||||
thq = Thread.new { q.pop }
|
||||
thsq = Thread.new { sq.pop }
|
||||
Thread.pass until thq.stop? && thsq.stop?
|
||||
|
||||
pid = fork do
|
||||
exit!(1) if q.num_waiting != 0
|
||||
exit!(2) if sq.num_waiting != 0
|
||||
exit!(6) unless q.empty?
|
||||
exit!(7) unless sq.empty?
|
||||
q.push :child_q
|
||||
sq.push :child_sq
|
||||
exit!(3) if q.pop != :child_q
|
||||
exit!(4) if sq.pop != :child_sq
|
||||
exit!(0)
|
||||
end
|
||||
_, s = Process.waitpid2(pid)
|
||||
assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]'
|
||||
|
||||
q.push :thq
|
||||
sq.push :thsq
|
||||
assert_equal :thq, thq.value
|
||||
assert_equal :thsq, thsq.value
|
||||
|
||||
sq.push(1)
|
||||
th = Thread.new { q.pop; sq.pop }
|
||||
thsq = Thread.new { sq.push(2) }
|
||||
Thread.pass until th.stop? && thsq.stop?
|
||||
pid = fork do
|
||||
exit!(1) if q.num_waiting != 0
|
||||
exit!(2) if sq.num_waiting != 0
|
||||
exit!(3) unless q.empty?
|
||||
exit!(4) if sq.empty?
|
||||
exit!(5) if sq.pop != 1
|
||||
exit!(0)
|
||||
end
|
||||
_, s = Process.waitpid2(pid)
|
||||
assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]'
|
||||
|
||||
assert_predicate thsq, :stop?
|
||||
assert_equal 1, sq.pop
|
||||
assert_same sq, thsq.value
|
||||
q.push('restart th')
|
||||
assert_equal 2, th.value
|
||||
end if Process.respond_to?(:fork)
|
||||
end
|
||||
|
|
2
thread.c
2
thread.c
|
@ -4217,6 +4217,8 @@ rb_thread_atfork_internal(rb_thread_t *th, void (*atfork)(rb_thread_t *, const r
|
|||
}
|
||||
rb_vm_living_threads_init(vm);
|
||||
rb_vm_living_threads_insert(vm, th);
|
||||
vm->fork_gen++;
|
||||
|
||||
vm->sleeper = 0;
|
||||
clear_coverage();
|
||||
}
|
||||
|
|
|
@ -4,6 +4,14 @@
|
|||
static VALUE rb_cMutex, rb_cQueue, rb_cSizedQueue, rb_cConditionVariable;
|
||||
static VALUE rb_eClosedQueueError;
|
||||
|
||||
/*
|
||||
* keep these globally so we can walk and reinitialize them at fork
|
||||
* in the child process
|
||||
*/
|
||||
static LIST_HEAD(szqueue_list);
|
||||
static LIST_HEAD(queue_list);
|
||||
static LIST_HEAD(condvar_list);
|
||||
|
||||
/* sync_waiter is always on-stack */
|
||||
struct sync_waiter {
|
||||
rb_thread_t *th;
|
||||
|
@ -556,6 +564,7 @@ void rb_mutex_allow_trap(VALUE self, int val)
|
|||
#define queue_waitq(q) UNALIGNED_MEMBER_PTR(q, waitq)
|
||||
PACKED_STRUCT_UNALIGNED(struct rb_queue {
|
||||
struct list_head waitq;
|
||||
rb_serial_t fork_gen;
|
||||
const VALUE que;
|
||||
int num_waiting;
|
||||
});
|
||||
|
@ -601,12 +610,29 @@ queue_alloc(VALUE klass)
|
|||
return obj;
|
||||
}
|
||||
|
||||
static int
|
||||
queue_fork_check(struct rb_queue *q)
|
||||
{
|
||||
rb_serial_t fork_gen = GET_VM()->fork_gen;
|
||||
|
||||
if (q->fork_gen == fork_gen) {
|
||||
return 0;
|
||||
}
|
||||
/* forked children can't reach into parent thread stacks */
|
||||
q->fork_gen = fork_gen;
|
||||
list_head_init(queue_waitq(q));
|
||||
q->num_waiting = 0;
|
||||
return 1;
|
||||
}
|
||||
|
||||
static struct rb_queue *
|
||||
queue_ptr(VALUE obj)
|
||||
{
|
||||
struct rb_queue *q;
|
||||
|
||||
TypedData_Get_Struct(obj, struct rb_queue, &queue_data_type, q);
|
||||
queue_fork_check(q);
|
||||
|
||||
return q;
|
||||
}
|
||||
|
||||
|
@ -649,6 +675,11 @@ szqueue_ptr(VALUE obj)
|
|||
struct rb_szqueue *sq;
|
||||
|
||||
TypedData_Get_Struct(obj, struct rb_szqueue, &szqueue_data_type, sq);
|
||||
if (queue_fork_check(&sq->q)) {
|
||||
list_head_init(szqueue_pushq(sq));
|
||||
sq->num_waiting_push = 0;
|
||||
}
|
||||
|
||||
return sq;
|
||||
}
|
||||
|
||||
|
@ -885,7 +916,7 @@ queue_do_pop(VALUE self, struct rb_queue *q, int should_block)
|
|||
list_add_tail(&qw.as.q->waitq, &qw.w.node);
|
||||
qw.as.q->num_waiting++;
|
||||
|
||||
rb_ensure(queue_sleep, Qfalse, queue_sleep_done, (VALUE)&qw);
|
||||
rb_ensure(queue_sleep, self, queue_sleep_done, (VALUE)&qw);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1127,7 +1158,7 @@ rb_szqueue_push(int argc, VALUE *argv, VALUE self)
|
|||
list_add_tail(pushq, &qw.w.node);
|
||||
sq->num_waiting_push++;
|
||||
|
||||
rb_ensure(queue_sleep, Qfalse, szqueue_sleep_done, (VALUE)&qw);
|
||||
rb_ensure(queue_sleep, self, szqueue_sleep_done, (VALUE)&qw);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1228,9 +1259,9 @@ rb_szqueue_empty_p(VALUE self)
|
|||
|
||||
|
||||
/* ConditionalVariable */
|
||||
/* TODO: maybe this can be IMEMO */
|
||||
struct rb_condvar {
|
||||
struct list_head waitq;
|
||||
rb_serial_t fork_gen;
|
||||
};
|
||||
|
||||
/*
|
||||
|
@ -1277,9 +1308,15 @@ static struct rb_condvar *
|
|||
condvar_ptr(VALUE self)
|
||||
{
|
||||
struct rb_condvar *cv;
|
||||
rb_serial_t fork_gen = GET_VM()->fork_gen;
|
||||
|
||||
TypedData_Get_Struct(self, struct rb_condvar, &cv_data_type, cv);
|
||||
|
||||
/* forked children can't reach into parent thread stacks */
|
||||
if (cv->fork_gen != fork_gen) {
|
||||
list_head_init(&cv->waitq);
|
||||
}
|
||||
|
||||
return cv;
|
||||
}
|
||||
|
||||
|
|
32
variable.c
32
variable.c
|
@ -20,6 +20,7 @@
|
|||
#include "ccan/list/list.h"
|
||||
#include "id_table.h"
|
||||
#include "debug_counter.h"
|
||||
#include "vm_core.h"
|
||||
|
||||
struct rb_id_table *rb_global_tbl;
|
||||
static ID autoload, classpath, tmp_classpath, classid;
|
||||
|
@ -1859,6 +1860,7 @@ struct autoload_data_i {
|
|||
rb_const_flag_t flag;
|
||||
VALUE value;
|
||||
struct autoload_state *state; /* points to on-stack struct */
|
||||
rb_serial_t fork_gen;
|
||||
};
|
||||
|
||||
static void
|
||||
|
@ -1881,8 +1883,18 @@ static const rb_data_type_t autoload_data_i_type = {
|
|||
0, 0, RUBY_TYPED_FREE_IMMEDIATELY
|
||||
};
|
||||
|
||||
#define check_autoload_data(av) \
|
||||
(struct autoload_data_i *)rb_check_typeddata((av), &autoload_data_i_type)
|
||||
static struct autoload_data_i *
|
||||
get_autoload_data(VALUE av)
|
||||
{
|
||||
struct autoload_data_i *ele = rb_check_typeddata(av, &autoload_data_i_type);
|
||||
|
||||
/* do not reach across stack for ->state after forking: */
|
||||
if (ele && ele->state && ele->fork_gen != GET_VM()->fork_gen) {
|
||||
ele->state = 0;
|
||||
ele->fork_gen = 0;
|
||||
}
|
||||
return ele;
|
||||
}
|
||||
|
||||
RUBY_FUNC_EXPORTED void
|
||||
rb_autoload(VALUE mod, ID id, const char *file)
|
||||
|
@ -1982,7 +1994,7 @@ check_autoload_required(VALUE mod, ID id, const char **loadingpath)
|
|||
const char *loading;
|
||||
int safe;
|
||||
|
||||
if (!(load = autoload_data(mod, id)) || !(ele = check_autoload_data(load))) {
|
||||
if (!(load = autoload_data(mod, id)) || !(ele = get_autoload_data(load))) {
|
||||
return 0;
|
||||
}
|
||||
file = ele->feature;
|
||||
|
@ -2020,7 +2032,7 @@ rb_autoloading_value(VALUE mod, ID id, VALUE* value, rb_const_flag_t *flag)
|
|||
VALUE load;
|
||||
struct autoload_data_i *ele;
|
||||
|
||||
if (!(load = autoload_data(mod, id)) || !(ele = check_autoload_data(load))) {
|
||||
if (!(load = autoload_data(mod, id)) || !(ele = get_autoload_data(load))) {
|
||||
return 0;
|
||||
}
|
||||
if (ele->state && ele->state->thread == rb_thread_current()) {
|
||||
|
@ -2087,8 +2099,9 @@ autoload_reset(VALUE arg)
|
|||
int need_wakeups = 0;
|
||||
|
||||
if (state->ele->state == state) {
|
||||
need_wakeups = 1;
|
||||
state->ele->state = 0;
|
||||
need_wakeups = 1;
|
||||
state->ele->state = 0;
|
||||
state->ele->fork_gen = 0;
|
||||
}
|
||||
|
||||
/* At the last, move a value defined in autoload to constant table */
|
||||
|
@ -2170,7 +2183,7 @@ rb_autoload_load(VALUE mod, ID id)
|
|||
if (src && loading && strcmp(src, loading) == 0) return Qfalse;
|
||||
|
||||
/* set ele->state for a marker of autoloading thread */
|
||||
if (!(ele = check_autoload_data(load))) {
|
||||
if (!(ele = get_autoload_data(load))) {
|
||||
return Qfalse;
|
||||
}
|
||||
|
||||
|
@ -2180,6 +2193,7 @@ rb_autoload_load(VALUE mod, ID id)
|
|||
state.thread = rb_thread_current();
|
||||
if (!ele->state) {
|
||||
ele->state = &state;
|
||||
ele->fork_gen = GET_VM()->fork_gen;
|
||||
|
||||
/*
|
||||
* autoload_reset will wake up any threads added to this
|
||||
|
@ -2217,7 +2231,7 @@ rb_autoload_p(VALUE mod, ID id)
|
|||
}
|
||||
load = check_autoload_required(mod, id, 0);
|
||||
if (!load) return Qnil;
|
||||
return (ele = check_autoload_data(load)) ? ele->feature : Qnil;
|
||||
return (ele = get_autoload_data(load)) ? ele->feature : Qnil;
|
||||
}
|
||||
|
||||
void
|
||||
|
@ -2646,7 +2660,7 @@ current_autoload_data(VALUE mod, ID id)
|
|||
struct autoload_data_i *ele;
|
||||
VALUE load = autoload_data(mod, id);
|
||||
if (!load) return 0;
|
||||
ele = check_autoload_data(load);
|
||||
ele = get_autoload_data(load);
|
||||
if (!ele) return 0;
|
||||
/* for autoloading thread, keep the defined value to autoloading storage */
|
||||
if (ele->state && (ele->state->thread == rb_thread_current())) {
|
||||
|
|
|
@ -1,10 +1,10 @@
|
|||
#define RUBY_VERSION "2.5.4"
|
||||
#define RUBY_RELEASE_DATE "2019-01-20"
|
||||
#define RUBY_PATCHLEVEL 137
|
||||
#define RUBY_RELEASE_DATE "2019-01-23"
|
||||
#define RUBY_PATCHLEVEL 138
|
||||
|
||||
#define RUBY_RELEASE_YEAR 2019
|
||||
#define RUBY_RELEASE_MONTH 1
|
||||
#define RUBY_RELEASE_DAY 20
|
||||
#define RUBY_RELEASE_DAY 23
|
||||
|
||||
#include "ruby/version.h"
|
||||
|
||||
|
|
Loading…
Reference in New Issue