Skip to content

Commit 7d8480f

Browse files
committed
Use EC saved in GC for root marking
Since EC is thread-local, we previously used rb_gc_worker_thread_set_vm_context in MMTk worker threads to temporarily set the EC. However, this was inelegant and also occasionally caused crashes when marking threads/fibers for the current EC since it will mark the current machine stack twice (once during root marking and once for the fiber). However, since the machine stack is actively being used, the contents may be different when marking the fiber. Since all objects on the machine stack are pinned, this may cause an unpinned object to be pinned, which is not allowed in Immix. The following crash can be observed: Object 0x200fffbc7d8 is trying to pin 0x200ffc80188 0: mmtk_ruby::handle_gc_thread_panic 1: mmtk_ruby::set_panic_hook::{{closure}} 2: <alloc::boxed::Box<dyn for<'a, 'b> core::ops::function::Fn<(&'a std::panic::PanicHookInfo<'b>,), Output = ()> + core::marker::Sync + core::marker::Send> as core::ops::function::Fn<(&std::panic::PanicHookInfo,)>>::call at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/alloc/src/boxed.rs:2254:9 3: std::panicking::panic_with_hook at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/panicking.rs:833:13 4: std::panicking::panic_handler::{closure#0} at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/panicking.rs:698:13 5: std::sys::backtrace::__rust_end_short_backtrace::<std::panicking::panic_handler::{closure#0}, !> at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/sys/backtrace.rs:182:18 6: __rustc::rust_begin_unwind at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/panicking.rs:689:5 7: core::panicking::panic_fmt at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/core/src/panicking.rs:80:14 8: <mmtk_ruby::scanning::VMScanning as mmtk::vm::scanning::Scanning<mmtk_ruby::Ruby>>::scan_object_and_trace_edges::{{closure}} 9: mmtk_ruby::abi::ObjectClosure::c_function_registered 10: rb_mmtk_call_object_closure at gc/mmtk/mmtk.c:976:19 11: rb_gc_impl_mark_and_pin at gc/mmtk/mmtk.c:1008:5 12: rb_gc_impl_mark_and_pin at gc/mmtk/mmtk.c:1004:1 13: gc_mark_maybe_internal at gc.c:2908:5 14: gc_mark_maybe_internal at gc.c:2906:1 15: gc_mark_maybe_each_location at gc.c:2939:5 16: gc_mark_maybe_each_location at gc.c:2937:1 17: each_location at gc.c:2924:9 18: each_location_ptr at gc.c:2933:5 19: each_location_ptr at gc.c:2930:1 20: rb_gc_mark_machine_context at gc.c:3200:5 21: rb_execution_context_mark at vm.c:3768:9 22: cont_mark at cont.c:1155:5 23: fiber_mark at cont.c:1284:5 24: rb_mmtk_call_gc_mark_children at gc/mmtk/mmtk.c:318:5 25: <mmtk_ruby::scanning::VMScanning as mmtk::vm::scanning::Scanning<mmtk_ruby::Ruby>>::scan_object_and_trace_edges::{{closure}}
1 parent 1c39cc8 commit 7d8480f

7 files changed

Lines changed: 42 additions & 10 deletions

File tree

gc.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,14 @@ rb_gc_get_ractor_newobj_cache(void)
191191
return GET_RACTOR()->newobj_cache;
192192
}
193193

194-
#if USE_MODULAR_GC
195194
void
196195
rb_gc_initialize_vm_context(struct rb_gc_vm_context *context)
197196
{
198197
rb_native_mutex_initialize(&context->lock);
199198
context->ec = GET_EC();
200199
}
201200

201+
#if USE_MODULAR_GC
202202
void
203203
rb_gc_worker_thread_set_vm_context(struct rb_gc_vm_context *context)
204204
{
@@ -626,6 +626,7 @@ typedef struct gc_function_map {
626626
void (*config_set)(void *objspace_ptr, VALUE hash);
627627
void (*stress_set)(void *objspace_ptr, VALUE flag);
628628
VALUE (*stress_get)(void *objspace_ptr);
629+
struct rb_gc_vm_context *(*get_vm_context)(void *objspace_ptr);
629630
// Object allocation
630631
VALUE (*new_obj)(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size);
631632
size_t (*obj_slot_size)(VALUE obj);
@@ -804,6 +805,7 @@ ruby_modular_gc_init(void)
804805
load_modular_gc_func(config_get);
805806
load_modular_gc_func(stress_set);
806807
load_modular_gc_func(stress_get);
808+
load_modular_gc_func(get_vm_context);
807809
// Object allocation
808810
load_modular_gc_func(new_obj);
809811
load_modular_gc_func(obj_slot_size);
@@ -891,6 +893,7 @@ ruby_modular_gc_init(void)
891893
# define rb_gc_impl_config_set rb_gc_functions.config_set
892894
# define rb_gc_impl_stress_set rb_gc_functions.stress_set
893895
# define rb_gc_impl_stress_get rb_gc_functions.stress_get
896+
# define rb_gc_impl_get_vm_context rb_gc_functions.get_vm_context
894897
// Object allocation
895898
# define rb_gc_impl_new_obj rb_gc_functions.new_obj
896899
# define rb_gc_impl_obj_slot_size rb_gc_functions.obj_slot_size
@@ -3238,10 +3241,23 @@ gc_declarative_marking_p(const rb_data_type_t *type)
32383241
return (type->flags & RUBY_TYPED_DECL_MARKING) != 0;
32393242
}
32403243

3244+
rb_execution_context_t *
3245+
rb_gc_get_ec(void)
3246+
{
3247+
void *objspace = rb_gc_get_objspace();
3248+
3249+
if (RB_LIKELY(rb_gc_impl_during_gc_p(objspace))) {
3250+
return rb_gc_impl_get_vm_context(objspace)->ec;
3251+
}
3252+
else {
3253+
return GET_EC();
3254+
}
3255+
}
3256+
32413257
void
32423258
rb_gc_mark_roots(void *objspace, const char **categoryp)
32433259
{
3244-
rb_execution_context_t *ec = GET_EC();
3260+
rb_execution_context_t *ec = rb_gc_get_ec();
32453261
rb_vm_t *vm = rb_ec_vm_ptr(ec);
32463262

32473263
#define MARK_CHECKPOINT(category) do { \

gc/default/default.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,8 @@ typedef struct rb_objspace {
683683
int sweeping_heap_count;
684684

685685
int fork_vm_lock_lev;
686+
687+
struct rb_gc_vm_context vm_context;
686688
} rb_objspace_t;
687689

688690
#ifndef HEAP_PAGE_ALIGN_LOG
@@ -1652,6 +1654,14 @@ rb_gc_impl_garbage_object_p(void *objspace_ptr, VALUE ptr)
16521654
!RVALUE_MARKED(objspace, ptr);
16531655
}
16541656

1657+
struct rb_gc_vm_context *
1658+
rb_gc_impl_get_vm_context(void *objspace_ptr)
1659+
{
1660+
rb_objspace_t *objspace = objspace_ptr;
1661+
1662+
return &objspace->vm_context;
1663+
}
1664+
16551665
static void free_stack_chunks(mark_stack_t *);
16561666
static void mark_stack_free_cache(mark_stack_t *);
16571667
static void heap_page_free(rb_objspace_t *objspace, struct heap_page *page);
@@ -6767,6 +6777,8 @@ gc_marking_enter(rb_objspace_t *objspace)
67676777
if (MEASURE_GC) {
67686778
gc_clock_start(&objspace->profile.marking_start_time);
67696779
}
6780+
6781+
rb_gc_initialize_vm_context(&objspace->vm_context);
67706782
}
67716783

67726784
static void

gc/gc.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,13 @@
1111
*/
1212
#include "ruby/ruby.h"
1313

14-
#if USE_MODULAR_GC
1514
#include "ruby/thread_native.h"
1615

1716
struct rb_gc_vm_context {
1817
rb_nativethread_lock_t lock;
1918

2019
struct rb_execution_context_struct *ec;
2120
};
22-
#endif
2321

2422
typedef int (*vm_table_foreach_callback_func)(VALUE value, void *data);
2523
typedef int (*vm_table_update_callback_func)(VALUE *value, void *data);

gc/gc_impl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ GC_IMPL_FN void rb_gc_impl_stress_set(void *objspace_ptr, VALUE flag);
5454
GC_IMPL_FN VALUE rb_gc_impl_stress_get(void *objspace_ptr);
5555
GC_IMPL_FN VALUE rb_gc_impl_config_get(void *objspace_ptr);
5656
GC_IMPL_FN void rb_gc_impl_config_set(void *objspace_ptr, VALUE hash);
57+
GC_IMPL_FN struct rb_gc_vm_context *rb_gc_impl_get_vm_context(void *objspace_ptr);
5758
// Object allocation
5859
GC_IMPL_FN VALUE rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size);
5960
GC_IMPL_FN size_t rb_gc_impl_obj_slot_size(VALUE obj);

gc/mmtk/mmtk.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,7 @@ rb_mmtk_scan_gc_roots(void)
253253
{
254254
struct objspace *objspace = rb_gc_get_objspace();
255255

256-
// FIXME: Make `rb_gc_mark_roots` aware that the current thread may not have EC.
257-
// See: https://github.com/ruby/mmtk/issues/22
258-
rb_gc_worker_thread_set_vm_context(&objspace->vm_context);
259256
rb_gc_mark_roots(objspace, NULL);
260-
rb_gc_worker_thread_unset_vm_context(&objspace->vm_context);
261257
}
262258

263259
static int
@@ -784,6 +780,14 @@ rb_gc_impl_config_set(void *objspace_ptr, VALUE hash)
784780
// TODO
785781
}
786782

783+
struct rb_gc_vm_context *
784+
rb_gc_impl_get_vm_context(void *objspace_ptr)
785+
{
786+
struct objspace *objspace = objspace_ptr;
787+
788+
return &objspace->vm_context;
789+
}
790+
787791
// Object allocation
788792

789793
static VALUE

internal/gc.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ RUBY_ATTR_MALLOC void *rb_xmalloc_mul_add_mul(size_t, size_t, size_t, size_t);
199199
RUBY_ATTR_MALLOC void *rb_xcalloc_mul_add_mul(size_t, size_t, size_t, size_t);
200200
void rb_gc_obj_id_moved(VALUE obj);
201201
void rb_gc_register_pinning_obj(VALUE obj);
202+
rb_execution_context_t *rb_gc_get_ec(void);
202203

203204
void *rb_gc_ractor_cache_alloc(rb_ractor_t *ractor);
204205
void rb_gc_ractor_cache_free(void *cache);

vm.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3764,8 +3764,8 @@ rb_execution_context_mark(const rb_execution_context_t *ec)
37643764

37653765
/* mark machine stack */
37663766
if (ec->machine.stack_start && ec->machine.stack_end &&
3767-
ec != GET_EC() /* marked for current ec at the first stage of marking */
3768-
) {
3767+
/* marked for current ec at the first stage of marking */
3768+
ec != rb_gc_get_ec()) {
37693769
rb_gc_mark_machine_context(ec);
37703770
}
37713771

0 commit comments

Comments
 (0)