Skip to content

Make sure we never context switch while holding VM lock. #735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Make sure we never context switch while holding VM lock. #735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8887a8
Make sure we never context switch while holding VM lock.
luke-gruber Sep 10, 2025
d9e6507
Revert changes introduced in 2f6c694977dc0cdc4766a8a921e74290963c19a7
luke-gruber Sep 12, 2025
1e3e0c7
Add a test that fails without these lock_rec changes.
luke-gruber Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions depend
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4199,7 +4199,9 @@ encoding.$(OBJEXT): $(top_srcdir)/internal/gc.h
encoding.$(OBJEXT): $(top_srcdir)/internal/imemo.h
encoding.$(OBJEXT): $(top_srcdir)/internal/inits.h
encoding.$(OBJEXT): $(top_srcdir)/internal/load.h
encoding.$(OBJEXT): $(top_srcdir)/internal/namespace.h
encoding.$(OBJEXT): $(top_srcdir)/internal/object.h
encoding.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
encoding.$(OBJEXT): $(top_srcdir)/internal/serial.h
encoding.$(OBJEXT): $(top_srcdir)/internal/set_table.h
encoding.$(OBJEXT): $(top_srcdir)/internal/static_assert.h
Expand Down Expand Up @@ -13362,6 +13364,7 @@ re.$(OBJEXT): {$(VPATH)}backward/2/stdalign.h
re.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h
re.$(OBJEXT): {$(VPATH)}config.h
re.$(OBJEXT): {$(VPATH)}constant.h
re.$(OBJEXT): {$(VPATH)}debug_counter.h
re.$(OBJEXT): {$(VPATH)}defines.h
re.$(OBJEXT): {$(VPATH)}encindex.h
re.$(OBJEXT): {$(VPATH)}encoding.h
Expand Down Expand Up @@ -13545,6 +13548,7 @@ re.$(OBJEXT): {$(VPATH)}util.h
re.$(OBJEXT): {$(VPATH)}vm_core.h
re.$(OBJEXT): {$(VPATH)}vm_debug.h
re.$(OBJEXT): {$(VPATH)}vm_opts.h
re.$(OBJEXT): {$(VPATH)}vm_sync.h
regcomp.$(OBJEXT): $(hdrdir)/ruby.h
regcomp.$(OBJEXT): $(hdrdir)/ruby/ruby.h
regcomp.$(OBJEXT): {$(VPATH)}assert.h
Expand Down Expand Up @@ -17722,21 +17726,32 @@ time.$(OBJEXT): {$(VPATH)}timev.rbinc
time.$(OBJEXT): {$(VPATH)}util.h
time.$(OBJEXT): {$(VPATH)}vm_core.h
time.$(OBJEXT): {$(VPATH)}vm_opts.h
transcode.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h
transcode.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h
transcode.$(OBJEXT): $(CCAN_DIR)/list/list.h
transcode.$(OBJEXT): $(CCAN_DIR)/str/str.h
transcode.$(OBJEXT): $(hdrdir)/ruby/ruby.h
transcode.$(OBJEXT): $(top_srcdir)/internal/array.h
transcode.$(OBJEXT): $(top_srcdir)/internal/basic_operators.h
transcode.$(OBJEXT): $(top_srcdir)/internal/class.h
transcode.$(OBJEXT): $(top_srcdir)/internal/compilers.h
transcode.$(OBJEXT): $(top_srcdir)/internal/encoding.h
transcode.$(OBJEXT): $(top_srcdir)/internal/gc.h
transcode.$(OBJEXT): $(top_srcdir)/internal/imemo.h
transcode.$(OBJEXT): $(top_srcdir)/internal/inits.h
transcode.$(OBJEXT): $(top_srcdir)/internal/namespace.h
transcode.$(OBJEXT): $(top_srcdir)/internal/object.h
transcode.$(OBJEXT): $(top_srcdir)/internal/sanitizers.h
transcode.$(OBJEXT): $(top_srcdir)/internal/serial.h
transcode.$(OBJEXT): $(top_srcdir)/internal/set_table.h
transcode.$(OBJEXT): $(top_srcdir)/internal/static_assert.h
transcode.$(OBJEXT): $(top_srcdir)/internal/string.h
transcode.$(OBJEXT): $(top_srcdir)/internal/transcode.h
transcode.$(OBJEXT): $(top_srcdir)/internal/variable.h
transcode.$(OBJEXT): $(top_srcdir)/internal/vm.h
transcode.$(OBJEXT): $(top_srcdir)/internal/warnings.h
transcode.$(OBJEXT): {$(VPATH)}assert.h
transcode.$(OBJEXT): {$(VPATH)}atomic.h
transcode.$(OBJEXT): {$(VPATH)}backward/2/assume.h
transcode.$(OBJEXT): {$(VPATH)}backward/2/attributes.h
transcode.$(OBJEXT): {$(VPATH)}backward/2/bool.h
Expand Down Expand Up @@ -17905,15 +17920,24 @@ transcode.$(OBJEXT): {$(VPATH)}internal/value_type.h
transcode.$(OBJEXT): {$(VPATH)}internal/variable.h
transcode.$(OBJEXT): {$(VPATH)}internal/warning_push.h
transcode.$(OBJEXT): {$(VPATH)}internal/xmalloc.h
transcode.$(OBJEXT): {$(VPATH)}method.h
transcode.$(OBJEXT): {$(VPATH)}missing.h
transcode.$(OBJEXT): {$(VPATH)}node.h
transcode.$(OBJEXT): {$(VPATH)}onigmo.h
transcode.$(OBJEXT): {$(VPATH)}oniguruma.h
transcode.$(OBJEXT): {$(VPATH)}ruby_assert.h
transcode.$(OBJEXT): {$(VPATH)}ruby_atomic.h
transcode.$(OBJEXT): {$(VPATH)}rubyparser.h
transcode.$(OBJEXT): {$(VPATH)}shape.h
transcode.$(OBJEXT): {$(VPATH)}st.h
transcode.$(OBJEXT): {$(VPATH)}subst.h
transcode.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h
transcode.$(OBJEXT): {$(VPATH)}thread_native.h
transcode.$(OBJEXT): {$(VPATH)}transcode.c
transcode.$(OBJEXT): {$(VPATH)}transcode_data.h
transcode.$(OBJEXT): {$(VPATH)}vm_core.h
transcode.$(OBJEXT): {$(VPATH)}vm_debug.h
transcode.$(OBJEXT): {$(VPATH)}vm_opts.h
transcode.$(OBJEXT): {$(VPATH)}vm_sync.h
util.$(OBJEXT): $(hdrdir)/ruby/ruby.h
util.$(OBJEXT): $(top_srcdir)/internal/array.h
Expand Down
91 changes: 58 additions & 33 deletions encoding.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "ruby/encoding.h"
#include "ruby/util.h"
#include "ruby_assert.h"
#include "vm_core.h"
#include "vm_sync.h"
#include "ruby_atomic.h"

Expand Down Expand Up @@ -421,6 +422,7 @@ int
rb_enc_register(const char *name, rb_encoding *encoding)
{
int index;
bool set_const = false;

GLOBAL_ENC_TABLE_LOCKING(enc_table) {
index = enc_registered(enc_table, name);
Expand All @@ -439,9 +441,12 @@ rb_enc_register(const char *name, rb_encoding *encoding)
}
else {
index = enc_register(enc_table, name, encoding);
set_encoding_const(name, rb_enc_from_index(index));
set_const = true;
}
}
if (set_const) {
set_encoding_const(name, rb_enc_from_index(index));
}
return index;
}

Expand Down Expand Up @@ -472,13 +477,14 @@ rb_enc_registered(const char *name)
void
rb_encdb_declare(const char *name)
{
int idx;
GLOBAL_ENC_TABLE_LOCKING(enc_table) {
int idx = enc_registered(enc_table, name);
idx = enc_registered(enc_table, name);
if (idx < 0) {
idx = enc_register(enc_table, name, 0);
}
set_encoding_const(name, rb_enc_from_index(idx));
}
set_encoding_const(name, rb_enc_from_index(idx));
}

static void
Expand Down Expand Up @@ -537,7 +543,6 @@ enc_replicate(struct enc_table *enc_table, const char *name, rb_encoding *encodi
idx = enc_register(enc_table, name, encoding);
if (idx < 0) rb_raise(rb_eArgError, "invalid encoding name: %s", name);
set_base_encoding(enc_table, idx, encoding);
set_encoding_const(name, rb_enc_from_index(idx));
return idx;
}

Expand All @@ -552,7 +557,6 @@ enc_replicate_with_index(struct enc_table *enc_table, const char *name, rb_encod
}
if (idx >= 0) {
set_base_encoding(enc_table, idx, origenc);
set_encoding_const(name, rb_enc_from_index(idx));
}
else {
rb_raise(rb_eArgError, "failed to replicate encoding");
Expand All @@ -575,6 +579,8 @@ rb_encdb_replicate(const char *name, const char *orig)
r = enc_replicate_with_index(enc_table, name, rb_enc_from_index(origidx), idx);
}

set_encoding_const(name, rb_enc_from_index(r));

return r;
}

Expand All @@ -588,6 +594,7 @@ rb_define_dummy_encoding(const char *name)
rb_encoding *enc = enc_table->list[index].enc;
ENC_SET_DUMMY((rb_raw_encoding *)enc);
}
set_encoding_const(name, rb_enc_from_index(index));

return index;
}
Expand All @@ -605,6 +612,8 @@ rb_encdb_dummy(const char *name)
ENC_SET_DUMMY((rb_raw_encoding *)enc);
}

set_encoding_const(name, rb_enc_from_index(index));

return index;
}

Expand Down Expand Up @@ -671,18 +680,20 @@ enc_alias_internal(struct enc_table *enc_table, const char *alias, int idx)
}

static int
enc_alias(struct enc_table *enc_table, const char *alias, int idx)
enc_alias(struct enc_table *enc_table, const char *alias, int idx, bool *set_const)
{
if (!valid_encoding_name_p(alias)) return -1;
if (!enc_alias_internal(enc_table, alias, idx))
set_encoding_const(alias, enc_from_index(enc_table, idx));
if (!enc_alias_internal(enc_table, alias, idx)) {
*set_const = true;
}
return idx;
}

int
rb_enc_alias(const char *alias, const char *orig)
{
int idx, r;
bool set_const = false;
GLOBAL_ENC_TABLE_LOCKING(enc_table) {
enc_check_addable(enc_table, alias); // can raise
}
Expand All @@ -691,7 +702,11 @@ rb_enc_alias(const char *alias, const char *orig)
if (idx < 0) return -1;

GLOBAL_ENC_TABLE_LOCKING(enc_table) {
r = enc_alias(enc_table, alias, idx);
r = enc_alias(enc_table, alias, idx, &set_const);
}

if (set_const) {
set_encoding_const(alias, rb_enc_from_index(idx));
}

return r;
Expand All @@ -701,14 +716,18 @@ int
rb_encdb_alias(const char *alias, const char *orig)
{
int r;
bool set_const = false;

GLOBAL_ENC_TABLE_LOCKING(enc_table) {
int idx = enc_registered(enc_table, orig);

if (idx < 0) {
idx = enc_register(enc_table, orig, 0);
}
r = enc_alias(enc_table, alias, idx);
r = enc_alias(enc_table, alias, idx, &set_const);
}
if (set_const) {
set_encoding_const(alias, rb_enc_from_index(r));
}

return r;
Expand All @@ -717,34 +736,35 @@ rb_encdb_alias(const char *alias, const char *orig)
static void
rb_enc_init(struct enc_table *enc_table)
{
ASSERT_vm_locking();
enc_table_expand(enc_table, ENCODING_COUNT + 1);
if (!enc_table->names) {
enc_table->names = st_init_strcasetable_with_size(ENCODING_LIST_CAPA);
}
RB_VM_LOCKING() {
enc_table_expand(enc_table, ENCODING_COUNT + 1);
if (!enc_table->names) {
enc_table->names = st_init_strcasetable_with_size(ENCODING_LIST_CAPA);
}
#define OnigEncodingASCII_8BIT OnigEncodingASCII
#define ENC_REGISTER(enc) enc_register_at(enc_table, ENCINDEX_##enc, rb_enc_name(&OnigEncoding##enc), &OnigEncoding##enc)
ENC_REGISTER(ASCII_8BIT);
ENC_REGISTER(UTF_8);
ENC_REGISTER(US_ASCII);
global_enc_ascii = enc_table->list[ENCINDEX_ASCII_8BIT].enc;
global_enc_utf_8 = enc_table->list[ENCINDEX_UTF_8].enc;
global_enc_us_ascii = enc_table->list[ENCINDEX_US_ASCII].enc;
ENC_REGISTER(ASCII_8BIT);
ENC_REGISTER(UTF_8);
ENC_REGISTER(US_ASCII);
global_enc_ascii = enc_table->list[ENCINDEX_ASCII_8BIT].enc;
global_enc_utf_8 = enc_table->list[ENCINDEX_UTF_8].enc;
global_enc_us_ascii = enc_table->list[ENCINDEX_US_ASCII].enc;
#undef ENC_REGISTER
#undef OnigEncodingASCII_8BIT
#define ENCDB_REGISTER(name, enc) enc_register_at(enc_table, ENCINDEX_##enc, name, NULL)
ENCDB_REGISTER("UTF-16BE", UTF_16BE);
ENCDB_REGISTER("UTF-16LE", UTF_16LE);
ENCDB_REGISTER("UTF-32BE", UTF_32BE);
ENCDB_REGISTER("UTF-32LE", UTF_32LE);
ENCDB_REGISTER("UTF-16", UTF_16);
ENCDB_REGISTER("UTF-32", UTF_32);
ENCDB_REGISTER("UTF8-MAC", UTF8_MAC);

ENCDB_REGISTER("EUC-JP", EUC_JP);
ENCDB_REGISTER("Windows-31J", Windows_31J);
ENCDB_REGISTER("UTF-16BE", UTF_16BE);
ENCDB_REGISTER("UTF-16LE", UTF_16LE);
ENCDB_REGISTER("UTF-32BE", UTF_32BE);
ENCDB_REGISTER("UTF-32LE", UTF_32LE);
ENCDB_REGISTER("UTF-16", UTF_16);
ENCDB_REGISTER("UTF-32", UTF_32);
ENCDB_REGISTER("UTF8-MAC", UTF8_MAC);

ENCDB_REGISTER("EUC-JP", EUC_JP);
ENCDB_REGISTER("Windows-31J", Windows_31J);
#undef ENCDB_REGISTER
enc_table->count = ENCINDEX_BUILTIN_MAX;
enc_table->count = ENCINDEX_BUILTIN_MAX;
}
}

rb_encoding *
Expand Down Expand Up @@ -865,7 +885,11 @@ int
rb_enc_find_index(const char *name)
{
int i;
ASSERT_vm_unlocking(); // it needs to be unlocked so it can call `load_encoding` if necessary
#if RUBY_DEBUG > 0
if (rb_multi_ractor_p() || !rb_enc_registered(name)) {
ASSERT_vm_unlocking(); // it needs to be unlocked so it can call `load_encoding` if necessary
}
#endif
GLOBAL_ENC_TABLE_LOCKING(enc_table) {
i = enc_registered(enc_table, name);
}
Expand Down Expand Up @@ -1812,6 +1836,7 @@ set_default_internal(VALUE klass, VALUE encoding)
static void
set_encoding_const(const char *name, rb_encoding *enc)
{
ASSERT_vm_unlocking();
VALUE encoding = rb_enc_from_encoding(enc);
char *s = (char *)name;
int haslower = 0, hasupper = 0, valid = 0;
Expand Down
3 changes: 3 additions & 0 deletions eval_intern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ extern int select_large_fdset(int, fd_set *, fd_set *, fd_set *, struct timeval

#include <sys/stat.h>

int ruby_thread_has_gvl_p(void);

#define EC_PUSH_TAG(ec) do { \
VM_ASSERT(ruby_thread_has_gvl_p()); \
rb_execution_context_t * const _ec = (ec); \
struct rb_vm_tag _tag; \
_tag.state = TAG_NONE; \
Expand Down
13 changes: 3 additions & 10 deletions gc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5081,19 +5081,12 @@ rb_memerror(void)
rb_execution_context_t *ec = GET_EC();
VALUE exc = GET_VM()->special_exceptions[ruby_error_nomemory];

if (!exc ||
rb_ec_raised_p(ec, RAISED_NOMEMORY) ||
rb_ec_vm_lock_rec(ec) != ec->tag->lock_rec) {
if (!exc || rb_ec_raised_p(ec, RAISED_NOMEMORY)) {
fprintf(stderr, "[FATAL] failed to allocate memory\n");
exit(EXIT_FAILURE);
}
if (rb_ec_raised_p(ec, RAISED_NOMEMORY)) {
rb_ec_raised_clear(ec);
}
else {
rb_ec_raised_set(ec, RAISED_NOMEMORY);
exc = ruby_vm_special_exception_copy(exc);
}
rb_ec_raised_set(ec, RAISED_NOMEMORY);
exc = ruby_vm_special_exception_copy(exc);
ec->errinfo = exc;
EC_JUMP_TAG(ec, TAG_RAISE);
}
Expand Down
11 changes: 7 additions & 4 deletions gc/default/default.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2773,7 +2773,7 @@ rb_gc_impl_define_finalizer(void *objspace_ptr, VALUE obj, VALUE block)

for (i = 0; i < len; i++) {
VALUE recv = RARRAY_AREF(table, i);
if (rb_equal(recv, block)) {
if (rb_equal(recv, block)) { // TODO: unsafe, can context switch
RB_GC_VM_UNLOCK(lev);
return recv;
}
Expand Down Expand Up @@ -2839,8 +2839,8 @@ get_final(long i, void *data)
return RARRAY_AREF(table, i + 1);
}

static void
run_final(rb_objspace_t *objspace, VALUE zombie)
static int
run_final(rb_objspace_t *objspace, VALUE zombie, unsigned int lev)
{
if (RZOMBIE(zombie)->dfree) {
RZOMBIE(zombie)->dfree(RZOMBIE(zombie)->data);
Expand All @@ -2851,7 +2851,9 @@ run_final(rb_objspace_t *objspace, VALUE zombie)
FL_UNSET(zombie, FL_FINALIZE);
st_data_t table;
if (st_delete(finalizer_table, &key, &table)) {
RB_GC_VM_UNLOCK(lev);
rb_gc_run_obj_finalizer(RARRAY_AREF(table, 0), RARRAY_LEN(table) - 1, get_final, (void *)table);
lev = RB_GC_VM_LOCK();
}
else {
rb_bug("FL_FINALIZE flag is set, but finalizers are not found");
Expand All @@ -2860,6 +2862,7 @@ run_final(rb_objspace_t *objspace, VALUE zombie)
else {
GC_ASSERT(!st_lookup(finalizer_table, key, NULL));
}
return lev;
}

static void
Expand All @@ -2874,7 +2877,7 @@ finalize_list(rb_objspace_t *objspace, VALUE zombie)

int lev = RB_GC_VM_LOCK();

run_final(objspace, zombie);
lev = run_final(objspace, zombie, lev);
{
GC_ASSERT(BUILTIN_TYPE(zombie) == T_ZOMBIE);
GC_ASSERT(page->heap->final_slots_count > 0);
Expand Down
Loading
Loading