From 45b8e103e5df97bc76d27d10982942ac7285654a Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Mon, 14 Nov 2011 17:56:42 -0700 Subject: [PATCH] places: fix GC of unreceived place-channel messages An unreceived message can have a reference to a master-allocated value, in which case that value must be marked. This marking is implemented by embedding a linked link within the message memory. --- collects/tests/racket/place-msg-gc.rkt | 14 ++++ src/racket/src/mzmark_place.inc | 30 +++++++ src/racket/src/mzmarksrc.c | 15 ++++ src/racket/src/place.c | 104 +++++++++++++++++-------- src/racket/src/schpriv.h | 1 + 5 files changed, 130 insertions(+), 34 deletions(-) create mode 100644 collects/tests/racket/place-msg-gc.rkt diff --git a/collects/tests/racket/place-msg-gc.rkt b/collects/tests/racket/place-msg-gc.rkt new file mode 100644 index 0000000000..e2578295d2 --- /dev/null +++ b/collects/tests/racket/place-msg-gc.rkt @@ -0,0 +1,14 @@ +#lang racket + +;; Check that a master GC doesn't drop a byte string +;; that is referenced only as a pending channel message + +(define-values (i o) (place-channel)) + +(place-channel-put i (list (make-shared-bytes 10))) + +(for ([i 100]) + (make-shared-bytes (* 1024 1024))) + +(let ([l (place-channel-get o)]) + (bytes-ref (car l) 9)) diff --git a/src/racket/src/mzmark_place.inc b/src/racket/src/mzmark_place.inc index 2811f23c5f..7163206810 100644 --- a/src/racket/src/mzmark_place.inc +++ b/src/racket/src/mzmark_place.inc @@ -82,20 +82,50 @@ static int place_async_channel_val_SIZE(void *p, struct NewGC *gc) { static int place_async_channel_val_MARK(void *p, struct NewGC *gc) { Scheme_Place_Async_Channel *pac = (Scheme_Place_Async_Channel *)p; + Scheme_Object *pr; + int i, j, sz; gcMARK2(pac->msgs, gc); gcMARK2(pac->msg_memory, gc); + gcMARK2(pac->msg_chains, gc); gcMARK2(pac->wakeup_signal, gc); + /* mark master-allocated objects within each messages: */ + j = pac->out; + sz = pac->size; + for (i = pac->count; i--; ) { + pr = pac->msg_chains[j]; + while (pr) { + gcMARK2(SCHEME_CAR(pr), gc); + pr = SCHEME_CDR(pr); + } + j = ((j + 1) & sz); + } + return gcBYTES_TO_WORDS(sizeof(Scheme_Place_Async_Channel)); } static int place_async_channel_val_FIXUP(void *p, struct NewGC *gc) { Scheme_Place_Async_Channel *pac = (Scheme_Place_Async_Channel *)p; + Scheme_Object *pr; + int i, j, sz; gcFIXUP2(pac->msgs, gc); gcFIXUP2(pac->msg_memory, gc); + gcFIXUP2(pac->msg_chains, gc); gcFIXUP2(pac->wakeup_signal, gc); + /* mark master-allocated objects within each messages: */ + j = pac->out; + sz = pac->size; + for (i = pac->count; i--; ) { + pr = pac->msg_chains[j]; + while (pr) { + gcFIXUP2(SCHEME_CAR(pr), gc); + pr = SCHEME_CDR(pr); + } + j = ((j + 1) & sz); + } + return gcBYTES_TO_WORDS(sizeof(Scheme_Place_Async_Channel)); } diff --git a/src/racket/src/mzmarksrc.c b/src/racket/src/mzmarksrc.c index 0e1e542605..d49442af0e 100644 --- a/src/racket/src/mzmarksrc.c +++ b/src/racket/src/mzmarksrc.c @@ -1470,10 +1470,25 @@ place_val { place_async_channel_val { mark: Scheme_Place_Async_Channel *pac = (Scheme_Place_Async_Channel *)p; + Scheme_Object *pr; + int i, j, sz; gcMARK2(pac->msgs, gc); gcMARK2(pac->msg_memory, gc); + gcMARK2(pac->msg_chains, gc); gcMARK2(pac->wakeup_signal, gc); + /* mark master-allocated objects within each messages: */ + j = pac->out; + sz = pac->size; + for (i = pac->count; i--; ) { + pr = pac->msg_chains[j]; + while (pr) { + gcMARK2(SCHEME_CAR(pr), gc); + pr = SCHEME_CDR(pr); + } + j = ((j + 1) & sz); + } + size: gcBYTES_TO_WORDS(sizeof(Scheme_Place_Async_Channel)); } diff --git a/src/racket/src/place.c b/src/racket/src/place.c index 2b901333e0..f6c86c62a5 100644 --- a/src/racket/src/place.c +++ b/src/racket/src/place.c @@ -73,7 +73,8 @@ static void destroy_place_object_locks(Scheme_Place_Object *place_obj); #if defined(MZ_USE_PLACES) && defined(MZ_PRECISE_GC) static Scheme_Object *places_deep_copy_worker(Scheme_Object *so, Scheme_Hash_Table **ht, - int mode, int gcable, int can_raise_exn); + int mode, int gcable, int can_raise_exn, + Scheme_Object **master_chain); # define mzPDC_CHECK 0 # define mzPDC_COPY 1 # define mzPDC_UNCOPY 2 @@ -1052,17 +1053,19 @@ static Scheme_Object *place_p(int argc, Scheme_Object *args[]) return SAME_TYPE(SCHEME_TYPE(args[0]), scheme_place_type) ? scheme_true : scheme_false; } -static Scheme_Object *do_places_deep_copy(Scheme_Object *so, int mode, int gcable) { +static Scheme_Object *do_places_deep_copy(Scheme_Object *so, int mode, int gcable, + Scheme_Object **master_chain) +{ #if defined(MZ_USE_PLACES) && defined(MZ_PRECISE_GC) Scheme_Hash_Table *ht = NULL; - return places_deep_copy_worker(so, &ht, mode, gcable, gcable); + return places_deep_copy_worker(so, &ht, mode, gcable, gcable, master_chain); #else return so; #endif } Scheme_Object *places_deep_uncopy(Scheme_Object *so) { - return do_places_deep_copy(so, mzPDC_UNCOPY, 1); + return do_places_deep_copy(so, mzPDC_UNCOPY, 1, NULL); } static void bad_place_message(Scheme_Object *so) { @@ -1111,7 +1114,7 @@ static void push_duped_fd(Scheme_Object **fd_accumulators, intptr_t slot, intptr } } -static Scheme_Object *trivial_copy(Scheme_Object *so) +static Scheme_Object *trivial_copy(Scheme_Object *so, Scheme_Object **master_chain) { switch (SCHEME_TYPE(so)) { case scheme_integer_type: @@ -1125,6 +1128,15 @@ static Scheme_Object *trivial_copy(Scheme_Object *so) case scheme_fxvector_type: if (SHARED_ALLOCATEDP(so)) { scheme_hash_key(so); + if (master_chain) { + /* Keep track of all the master-allocated objects that are + in a message, so that the corresponding objects can be + marked during a master GC, in case on happens before the + message is received. */ + Scheme_Object *mc; + mc = scheme_make_raw_pair(so, *master_chain); + *master_chain = mc; + } return so; } } @@ -1133,11 +1145,13 @@ static Scheme_Object *trivial_copy(Scheme_Object *so) } static Scheme_Object *shallow_types_copy(Scheme_Object *so, Scheme_Hash_Table *ht, - Scheme_Object **fd_accumulators, intptr_t *delayed_errno, int mode, int can_raise_exn) { + Scheme_Object **fd_accumulators, intptr_t *delayed_errno, + int mode, int can_raise_exn, + Scheme_Object **master_chain) { Scheme_Object *new_so; int copy_mode = ((mode == mzPDC_COPY) || (mode == mzPDC_UNCOPY)); - new_so = trivial_copy(so); + new_so = trivial_copy(so, master_chain); if (new_so) return new_so; new_so = so; @@ -1153,6 +1167,12 @@ static Scheme_Object *shallow_types_copy(Scheme_Object *so, Scheme_Hash_Table *h ch->so.type = scheme_place_bi_channel_type; ch->sendch = ((Scheme_Place_Bi_Channel *)so)->sendch; ch->recvch = ((Scheme_Place_Bi_Channel *)so)->recvch; + if (master_chain) { + /* See setting of master_chain in trivial_copy(): */ + new_so = scheme_make_raw_pair((Scheme_Object *)ch->sendch, *master_chain); + new_so = scheme_make_raw_pair((Scheme_Object *)ch->recvch, new_so); + *master_chain = new_so; + } new_so = (Scheme_Object *)ch; } break; @@ -1170,8 +1190,8 @@ static Scheme_Object *shallow_types_copy(Scheme_Object *so, Scheme_Hash_Table *h Scheme_Object *d; n = scheme_rational_numerator(so); d = scheme_rational_denominator(so); - n = shallow_types_copy(n, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn); - d = shallow_types_copy(d, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn); + n = shallow_types_copy(n, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn, master_chain); + d = shallow_types_copy(d, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn, master_chain); new_so = scheme_make_rational(n, d); } break; @@ -1189,8 +1209,8 @@ static Scheme_Object *shallow_types_copy(Scheme_Object *so, Scheme_Hash_Table *h Scheme_Object *i; r = scheme_complex_real_part(so); i = scheme_complex_imaginary_part(so); - r = shallow_types_copy(r, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn); - i = shallow_types_copy(i, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn); + r = shallow_types_copy(r, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn, master_chain); + i = shallow_types_copy(i, NULL, fd_accumulators, delayed_errno, mode, can_raise_exn, master_chain); new_so = scheme_make_complex(r, i); } break; @@ -1279,12 +1299,14 @@ static Scheme_Object *shallow_types_copy(Scheme_Object *so, Scheme_Hash_Table *h o->type = scheme_cpointer_type; SCHEME_CPTR_FLAGS(o) |= 0x1; SCHEME_CPTR_VAL(o) = SCHEME_CPTR_VAL(so); - o2 = shallow_types_copy(SCHEME_CPTR_TYPE(so), NULL, fd_accumulators, delayed_errno, mode, can_raise_exn); + o2 = shallow_types_copy(SCHEME_CPTR_TYPE(so), NULL, fd_accumulators, delayed_errno, mode, + can_raise_exn, master_chain); SCHEME_CPTR_TYPE(o) = o2; new_so = o; } else { - (void)shallow_types_copy(SCHEME_CPTR_TYPE(so), NULL, fd_accumulators, delayed_errno, mode, can_raise_exn); + (void)shallow_types_copy(SCHEME_CPTR_TYPE(so), NULL, fd_accumulators, delayed_errno, mode, + can_raise_exn, master_chain); } } else { @@ -1319,7 +1341,7 @@ static Scheme_Object *shallow_types_copy(Scheme_Object *so, Scheme_Hash_Table *h ssfd->type = so->type; ssfd->fd = dupfd; portname = scheme_port_name(so); - tmp = shallow_types_copy(portname, ht, fd_accumulators, delayed_errno, mode, can_raise_exn); + tmp = shallow_types_copy(portname, ht, fd_accumulators, delayed_errno, mode, can_raise_exn, master_chain); ssfd->name = tmp; return (Scheme_Object *)ssfd; } @@ -1333,7 +1355,8 @@ static Scheme_Object *shallow_types_copy(Scheme_Object *so, Scheme_Hash_Table *h sffd = scheme_malloc_tagged(sizeof(Scheme_Serialized_File_FD)); sffd->so.type = scheme_serialized_file_fd_type; scheme_get_serialized_fd_flags(so, sffd); - tmp = shallow_types_copy(scheme_port_name(so), ht, fd_accumulators, delayed_errno, mode, can_raise_exn); + tmp = shallow_types_copy(scheme_port_name(so), ht, fd_accumulators, delayed_errno, mode, + can_raise_exn, master_chain); sffd->name = tmp; dupfd = scheme_dup_file(fd); if (dupfd == -1) { @@ -1590,7 +1613,8 @@ static MZ_INLINE Scheme_Object *inf_get(Scheme_Object **instack, int pos, uintpt in that case. Therefore, it must use its own stack implementation for recursion. */ static Scheme_Object *places_deep_copy_worker(Scheme_Object *so, Scheme_Hash_Table **ht, - int mode, int gcable, int can_raise_exn) { + int mode, int gcable, int can_raise_exn, + Scheme_Object **master_chain) { Scheme_Object *inf_stack = NULL; Scheme_Object *reg0 = NULL; uintptr_t inf_stack_depth = 0, inf_max_depth = 0; @@ -1635,7 +1659,7 @@ static Scheme_Object *places_deep_copy_worker(Scheme_Object *so, Scheme_Hash_Tab int ctr = 0; /* First, check for simple values that don't need to be hashed: */ - new_so = shallow_types_copy(so, *ht, &fd_accumulators, &delayed_errno, mode, can_raise_exn); + new_so = shallow_types_copy(so, *ht, &fd_accumulators, &delayed_errno, mode, can_raise_exn, master_chain); if (new_so) return new_so; if (*ht) { @@ -1662,7 +1686,7 @@ DEEP_DO: ctr++; so = GET_R0(); - new_so = trivial_copy(so); + new_so = trivial_copy(so, master_chain); if (new_so) RETURN; if (*ht) { @@ -1672,7 +1696,8 @@ DEEP_DO: } } - new_so = shallow_types_copy(so, *ht, &fd_accumulators, &delayed_errno, mode, can_raise_exn); + new_so = shallow_types_copy(so, *ht, &fd_accumulators, &delayed_errno, mode, + can_raise_exn, master_chain); if (new_so) RETURN; new_so = so; @@ -2323,32 +2348,32 @@ Scheme_Object *places_deep_copy_to_master(Scheme_Object *so) { void *original_gc; /* forces hash codes: */ - (void)places_deep_copy_worker(so, &ht, mzPDC_CHECK, 1, 1); + (void)places_deep_copy_worker(so, &ht, mzPDC_CHECK, 1, 1, NULL); ht = NULL; original_gc = GC_switch_to_master_gc(); scheme_start_atomic(); - o = places_deep_copy_worker(so, &ht, mzPDC_COPY, 1, 0); + o = places_deep_copy_worker(so, &ht, mzPDC_COPY, 1, 0, NULL); scheme_end_atomic_no_swap(); GC_switch_back_from_master(original_gc); return o; #else - return places_deep_copy_worker(so, &ht, mzPDC_COPY, 1, 1); + return places_deep_copy_worker(so, &ht, mzPDC_COPY, 1, 1, NULL); #endif } -Scheme_Object *scheme_places_serialize(Scheme_Object *so, void **msg_memory) { +static Scheme_Object *places_serialize(Scheme_Object *so, void **msg_memory, Scheme_Object **master_chain) { #if defined(MZ_USE_PLACES) && defined(MZ_PRECISE_GC) Scheme_Object *new_so; Scheme_Object *tmp; - new_so = trivial_copy(so); + new_so = trivial_copy(so, NULL); if (new_so) return new_so; GC_create_message_allocator(); - new_so = do_places_deep_copy(so, mzPDC_COPY, 0); + new_so = do_places_deep_copy(so, mzPDC_COPY, 0, master_chain); tmp = GC_finish_message_allocator(); (*msg_memory) = tmp; return new_so; @@ -2361,18 +2386,18 @@ Scheme_Object *scheme_places_deserialize(Scheme_Object *so, void *msg_memory) { #if defined(MZ_USE_PLACES) && defined(MZ_PRECISE_GC) Scheme_Object *new_so = so; - new_so = trivial_copy(so); + new_so = trivial_copy(so, NULL); if (new_so) return new_so; /* small messages are deemed to be < 1k, this could be tuned in either direction */ if (GC_message_objects_size(msg_memory) < 1024) { - new_so = do_places_deep_copy(so, mzPDC_UNCOPY, 1); + new_so = do_places_deep_copy(so, mzPDC_UNCOPY, 1, NULL); GC_dispose_short_message_allocator(msg_memory); } else { GC_adopt_message_allocator(msg_memory); #if !defined(SHARED_TABLES) - new_so = do_places_deep_copy(so, mzPDC_DESER, 1); + new_so = do_places_deep_copy(so, mzPDC_DESER, 1, NULL); #endif } return new_so; @@ -2417,7 +2442,7 @@ static Scheme_Object* place_allowed_p(int argc, Scheme_Object *args[]) { Scheme_Hash_Table *ht = NULL; - if (places_deep_copy_worker(args[0], &ht, mzPDC_CHECK, 1, 0)) + if (places_deep_copy_worker(args[0], &ht, mzPDC_CHECK, 1, 0, NULL)) return scheme_true; else return scheme_false; @@ -2490,7 +2515,7 @@ static void async_channel_finalize(void *p, void* data) { for (i = 0; i < ch->size ; i++) { ht = NULL; if (ch->msgs[i]) { - (void)places_deep_copy_worker(ch->msgs[i], &ht, mzPDC_CLEAN, 0, 0); + (void)places_deep_copy_worker(ch->msgs[i], &ht, mzPDC_CLEAN, 0, 0, NULL); ch->msgs[i] = NULL; } #ifdef MZ_PRECISE_GC @@ -2499,6 +2524,7 @@ static void async_channel_finalize(void *p, void* data) { } #endif ch->msg_memory[i] = NULL; + ch->msg_chains[i] = NULL; } ch->in = 0; ch->out = 0; @@ -2545,7 +2571,7 @@ static void async_channel_finalize(void *p, void* data) { } Scheme_Place_Async_Channel *place_async_channel_create() { - Scheme_Object **msgs; + Scheme_Object **msgs, **msg_chains; Scheme_Place_Async_Channel *ch; void **msg_memory; #ifdef MZ_PRECISE_GC @@ -2555,6 +2581,7 @@ Scheme_Place_Async_Channel *place_async_channel_create() { ch = GC_master_malloc_tagged(sizeof(Scheme_Place_Async_Channel)); msgs = GC_master_malloc(sizeof(Scheme_Object*) * 8); msg_memory = GC_master_malloc(sizeof(void*) * 8); + msg_chains = GC_master_malloc(sizeof(Scheme_Object*) * 8); ch->so.type = scheme_place_async_channel_type; ch->in = 0; @@ -2564,6 +2591,7 @@ Scheme_Place_Async_Channel *place_async_channel_create() { mzrt_mutex_create(&ch->lock); ch->msgs = msgs; ch->msg_memory = msg_memory; + ch->msg_chains = msg_chains; ch->wakeup_signal = NULL; #ifdef MZ_PRECISE_GC @@ -2632,26 +2660,28 @@ static Scheme_Object *GC_master_make_vector(int size) { static void place_async_send(Scheme_Place_Async_Channel *ch, Scheme_Object *uo) { void *msg_memory = NULL; - Scheme_Object *o; + Scheme_Object *o, *master_chain = NULL; intptr_t sz; int cnt; - o = scheme_places_serialize(uo, &msg_memory); + o = places_serialize(uo, &msg_memory, &master_chain); if (!o) bad_place_message(uo); mzrt_mutex_lock(ch->lock); { cnt = ch->count; if (ch->count == ch->size) { /* GROW QUEUE */ - Scheme_Object **new_msgs; + Scheme_Object **new_msgs, **new_chains; void **new_msg_memory; new_msgs = GC_master_malloc(sizeof(Scheme_Object*) * ch->size * 2); new_msg_memory = GC_master_malloc(sizeof(void*) * ch->size * 2); + new_chains = GC_master_malloc(sizeof(Scheme_Object*) * ch->size * 2); if (ch->out < ch->in) { memcpy(new_msgs, ch->msgs + ch->out, sizeof(Scheme_Object *) * (ch->in - ch->out)); memcpy(new_msg_memory, ch->msg_memory + ch->out, sizeof(void*) * (ch->in - ch->out)); + memcpy(new_chains, ch->msg_chains + ch->out, sizeof(void*) * (ch->in - ch->out)); } else { int s1 = (ch->size - ch->out); @@ -2660,10 +2690,14 @@ static void place_async_send(Scheme_Place_Async_Channel *ch, Scheme_Object *uo) memcpy(new_msg_memory, ch->msg_memory + ch->out, sizeof(void*) * s1); memcpy(new_msg_memory + s1, ch->msg_memory, sizeof(void*) * ch->in); + + memcpy(new_chains, ch->msg_chains + ch->out, sizeof(Scheme_Object *) * s1); + memcpy(new_chains + s1, ch->msg_chains, sizeof(Scheme_Object *) * ch->in); } ch->msgs = new_msgs; ch->msg_memory = new_msg_memory; + ch->msg_chains = new_chains; ch->in = ch->size; ch->out = 0; ch->size *= 2; @@ -2671,6 +2705,7 @@ static void place_async_send(Scheme_Place_Async_Channel *ch, Scheme_Object *uo) ch->msgs[ch->in] = o; ch->msg_memory[ch->in] = msg_memory; + ch->msg_chains[ch->in] = master_chain; ++ch->count; ch->in = ((ch->in + 1) % ch->size); @@ -2857,6 +2892,7 @@ static Scheme_Object *scheme_place_async_try_receive(Scheme_Place_Async_Channel ch->msgs[ch->out] = NULL; ch->msg_memory[ch->out] = NULL; + ch->msg_chains[ch->out] = NULL; --ch->count; ch->out = ((ch->out + 1) % ch->size); diff --git a/src/racket/src/schpriv.h b/src/racket/src/schpriv.h index 7e57086986..2e90e060da 100644 --- a/src/racket/src/schpriv.h +++ b/src/racket/src/schpriv.h @@ -3678,6 +3678,7 @@ typedef struct Scheme_Place_Async_Channel { #endif Scheme_Object **msgs; void **msg_memory; + Scheme_Object **msg_chains; intptr_t mem_size; intptr_t reported_size; /* size reported to master GC; avoid reporting too often */ void *wakeup_signal;