Skip to content

Commit cdc251c

Browse files
author
normal
committed
variable.c: additional locking around autoload
[ruby-core:70075] [ruby-core:71239] [Bug #11384] Note: this open-coding locking method may go into rb_mutex/rb_thread_shield types. It is smaller and simpler and based on the wait queue implementation of the Linux kernel. When/if we get rid of GVL, native mutexes may be used as-is. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@52332 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
1 parent 39715ad commit cdc251c

File tree

2 files changed

+88
-20
lines changed

2 files changed

+88
-20
lines changed

ChangeLog

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
Thu Oct 29 08:48:05 2015 Eric Wong <e@80x24.org>
2+
3+
* variable.c: additional locking around autoload
4+
[ruby-core:70075] [ruby-core:71239] [Bug #11384]
5+
16
Wed Oct 29 00:39:50 2015 Naohisa Goto <ngotogenome@gmail.com>
27

38
* test/rubygems/test_gem_commands_server_command.rb

variable.c

Lines changed: 83 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "ruby/util.h"
1717
#include "constant.h"
1818
#include "id.h"
19+
#include "ccan/list/list.h"
1920

2021
st_table *rb_global_tbl;
2122
static ID autoload, classpath, tmp_classpath, classid;
@@ -1896,6 +1897,7 @@ struct autoload_data_i {
18961897
int safe_level;
18971898
VALUE thread;
18981899
VALUE value;
1900+
struct list_head waitq_head; /* links autoload_state.waitq_node */
18991901
};
19001902

19011903
static void
@@ -2077,20 +2079,71 @@ autoload_const_set(VALUE arg)
20772079
return 0;/* ignored */
20782080
}
20792081

2082+
/* this is on the thread stack, not heap */
2083+
struct autoload_state {
2084+
struct autoload_data_i *ele;
2085+
VALUE mod;
2086+
VALUE result;
2087+
ID id;
2088+
struct list_node waitq_node; /* links autoload_data_i.waitq_head */
2089+
VALUE waiting_th;
2090+
};
2091+
20802092
static VALUE
20812093
autoload_require(VALUE arg)
20822094
{
2083-
struct autoload_data_i *ele = (struct autoload_data_i *)arg;
2084-
return rb_funcall(rb_vm_top_self(), rb_intern("require"), 1, ele->feature);
2095+
struct autoload_state *state = (struct autoload_state *)arg;
2096+
2097+
/* this may release GVL and switch threads: */
2098+
state->result = rb_funcall(rb_vm_top_self(), rb_intern("require"), 1,
2099+
state->ele->feature);
2100+
2101+
return state->result;
20852102
}
20862103

20872104
static VALUE
20882105
autoload_reset(VALUE arg)
20892106
{
2090-
struct autoload_data_i *ele = (struct autoload_data_i *)arg;
2091-
if (ele->thread == rb_thread_current()) {
2092-
ele->thread = Qnil;
2107+
struct autoload_state *state = (struct autoload_state *)arg;
2108+
int need_wakeups = 0;
2109+
2110+
if (state->ele->thread == rb_thread_current()) {
2111+
need_wakeups = 1;
2112+
state->ele->thread = Qnil;
2113+
}
2114+
2115+
/* At the last, move a value defined in autoload to constant table */
2116+
if (RTEST(state->result) && state->ele->value != Qundef) {
2117+
int safe_backup;
2118+
struct autoload_const_set_args args;
2119+
2120+
args.mod = state->mod;
2121+
args.id = state->id;
2122+
args.value = state->ele->value;
2123+
safe_backup = rb_safe_level();
2124+
rb_set_safe_level_force(state->ele->safe_level);
2125+
rb_ensure(autoload_const_set, (VALUE)&args,
2126+
reset_safe, (VALUE)safe_backup);
2127+
}
2128+
2129+
/* wakeup any waiters we had */
2130+
if (need_wakeups) {
2131+
struct autoload_state *cur, *nxt;
2132+
2133+
list_for_each_safe(&state->ele->waitq_head, cur, nxt, waitq_node) {
2134+
VALUE th = cur->waiting_th;
2135+
2136+
cur->waiting_th = Qfalse;
2137+
list_del(&cur->waitq_node);
2138+
2139+
/*
2140+
* cur is stored on the stack of cur->waiting_th,
2141+
* do not touch cur after waking up waiting_th
2142+
*/
2143+
rb_thread_wakeup_alive(th);
2144+
}
20932145
}
2146+
20942147
return 0;/* ignored */
20952148
}
20962149

@@ -2100,6 +2153,7 @@ rb_autoload_load(VALUE mod, ID id)
21002153
VALUE load, result;
21012154
const char *loading = 0, *src;
21022155
struct autoload_data_i *ele;
2156+
struct autoload_state state;
21032157

21042158
if (!autoload_defined_p(mod, id)) return Qfalse;
21052159
load = check_autoload_required(mod, id, &loading);
@@ -2111,26 +2165,35 @@ rb_autoload_load(VALUE mod, ID id)
21112165
if (!(ele = check_autoload_data(load))) {
21122166
return Qfalse;
21132167
}
2168+
2169+
state.ele = ele;
2170+
state.mod = mod;
2171+
state.id = id;
21142172
if (ele->thread == Qnil) {
21152173
ele->thread = rb_thread_current();
2174+
2175+
/*
2176+
* autoload_reset will wake up any threads added to this
2177+
* iff the GVL is released during autoload_require
2178+
*/
2179+
list_head_init(&ele->waitq_head);
21162180
}
2181+
else {
2182+
state.waiting_th = rb_thread_current();
2183+
list_add_tail(&ele->waitq_head, &state.waitq_node);
2184+
/*
2185+
* autoload_reset in other thread will resume us and remove us
2186+
* from the waitq list
2187+
*/
2188+
do {
2189+
rb_thread_sleep_deadly();
2190+
} while (state.waiting_th != Qfalse);
2191+
}
2192+
21172193
/* autoload_data_i can be deleted by another thread while require */
2118-
result = rb_ensure(autoload_require, (VALUE)ele, autoload_reset, (VALUE)ele);
2194+
result = rb_ensure(autoload_require, (VALUE)&state,
2195+
autoload_reset, (VALUE)&state);
21192196

2120-
if (RTEST(result)) {
2121-
/* At the last, move a value defined in autoload to constant table */
2122-
if (ele->value != Qundef) {
2123-
int safe_backup;
2124-
struct autoload_const_set_args args;
2125-
args.mod = mod;
2126-
args.id = id;
2127-
args.value = ele->value;
2128-
safe_backup = rb_safe_level();
2129-
rb_set_safe_level_force(ele->safe_level);
2130-
rb_ensure(autoload_const_set, (VALUE)&args,
2131-
reset_safe, (VALUE)safe_backup);
2132-
}
2133-
}
21342197
RB_GC_GUARD(load);
21352198
return result;
21362199
}

0 commit comments

Comments
 (0)