summaryrefslogtreecommitdiff
path: root/gc.c
diff options
context:
space:
mode:
authorKJ Tsanaktsidis <kj@kjtsanaktsidis.id.au>2023-11-19 22:54:57 +1100
committerKoichi Sasada <ko1@atdot.net>2023-12-10 15:00:37 +0900
commitf8effa209adb3ce050c100ffaffe6f3cc1508185 (patch)
treec0a672d5917a9917910679504d0f8b2d450088f7 /gc.c
parentaecbd66742f43ccfcac04ca4143fcc68ad834320 (diff)
Change the semantics of rb_postponed_job_register
Our current implementation of rb_postponed_job_register suffers from some safety issues that can lead to interpreter crashes (see bug #1991). Essentially, the issue is that jobs can be called with the wrong arguments. We made two attempts to fix this whilst keeping the promised semantics, but: * The first one involved masking/unmasking when flushing jobs, which was believed to be too expensive * The second one involved a lock-free, multi-producer, single-consumer ringbuffer, which was too complex The critical insight behind this third solution is that essentially the only user of these APIs are a) internal, or b) profiling gems. For a), none of the usages actually require variable data; they will work just fine with the preregistration interface. For b), generally profiling gems only call a single callback with a single piece of data (which is actually usually just zero) for the life of the program. The ringbuffer is complex because it needs to support multi-word inserts of job & data (which can't be atomic); but nobody actually even needs that functionality, really. So, this comit: * Introduces a pre-registration API for jobs, with a GVL-requiring rb_postponed_job_prereigster, which returns a handle which can be used with an async-signal-safe rb_postponed_job_trigger. * Deprecates rb_postponed_job_register (and re-implements it on top of the preregister function for compatability) * Moves all the internal usages of postponed job register pre-registration
Diffstat (limited to 'gc.c')
-rw-r--r--gc.c12
1 files changed, 9 insertions, 3 deletions
diff --git a/gc.c b/gc.c
index ce5ae34dc8..6050fd522a 100644
--- a/gc.c
+++ b/gc.c
@@ -952,6 +952,7 @@ typedef struct rb_objspace {
#endif
rb_darray(VALUE *) weak_references;
+ rb_postponed_job_handle_t finalize_deferred_pjob;
} rb_objspace_t;
@@ -1425,6 +1426,8 @@ PRINTF_ARGS(static void gc_report_body(int level, rb_objspace_t *objspace, const
static const char *obj_info(VALUE obj);
static const char *obj_type_name(VALUE obj);
+static void gc_finalize_deferred(void *dmy);
+
/*
* 1 - TSC (H/W Time Stamp Counter)
* 2 - getrusage
@@ -1906,6 +1909,10 @@ rb_objspace_alloc(void)
rb_objspace_t *objspace = calloc1(sizeof(rb_objspace_t));
objspace->flags.measure_gc = 1;
malloc_limit = gc_params.malloc_limit_min;
+ objspace->finalize_deferred_pjob = rb_postponed_job_preregister(gc_finalize_deferred, objspace);
+ if (objspace->finalize_deferred_pjob == POSTPONED_JOB_HANDLE_INVALID) {
+ rb_bug("Could not preregister postponed job for GC");
+ }
for (int i = 0; i < SIZE_POOL_COUNT; i++) {
rb_size_pool_t *size_pool = &size_pools[i];
@@ -4527,9 +4534,8 @@ gc_finalize_deferred(void *dmy)
static void
gc_finalize_deferred_register(rb_objspace_t *objspace)
{
- if (rb_postponed_job_register_one(0, gc_finalize_deferred, objspace) == 0) {
- rb_bug("gc_finalize_deferred_register: can't register finalizer.");
- }
+ /* will enqueue a call to gc_finalize_deferred */
+ rb_postponed_job_trigger(objspace->finalize_deferred_pjob);
}
static int pop_mark_stack(mark_stack_t *stack, VALUE *data);