From c162f73f9cfd0bb48675dc11bd56ac359451510b Mon Sep 17 00:00:00 2001 From: Kaz Kylheku Date: Thu, 5 Apr 2012 09:18:14 -0700 Subject: The mut macro should only be used for vectors or vector-like objects which hold direct references to other objects and must be used each time a mutation takes place. * eval.c (op_dohash): invocations of mut macro removed. Comment rewritten. * lib.c (sort_list): Use set macro for mutating assignment. Do not invoke mut on sorted list; it won't work anyway, because it doesn't mean what the code wants it to mean: that the list will be fully traversed during gc marking. --- ChangeLog | 14 ++++++++++++++ eval.c | 16 ++++++---------- lib.c | 8 ++++++-- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4ed8c6c5..4b3d95c6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2012-04-05 Kaz Kylheku + + The mut macro should only be used for vectors or vector-like objects + which hold direct references to other objects and must be used + each time a mutation takes place. + + * eval.c (op_dohash): invocations of mut macro removed. + Comment rewritten. + + * lib.c (sort_list): Use set macro for mutating assignment. + Do not invoke mut on sorted list; it won't work anyway, because + it doesn't mean what the code wants it to mean: that the list will be + fully traversed during gc marking. + 2012-04-05 Kaz Kylheku Bunch of fixes. diff --git a/eval.c b/eval.c index 4a44c194..32be8f2f 100644 --- a/eval.c +++ b/eval.c @@ -1035,17 +1035,13 @@ static val op_dohash(val form, val env) uw_block_begin (nil, result); - /* - * Avoid issuing set() operations in the loop; - * just tell GC that these variables are being mutated. - * TODO: This is not enough since gc can take place while we execute this - * loop. What we need is to conditionally re-establish this. - * GC needs to provide a way to let us know "has GC happened since ..." - */ - mut(keyvar); - mut(valvar); - while ((cell = hash_next(&iter)) != nil) { + /* These assignments are gc-safe, because keyvar and valvar + are newer objects than existing entries in the hash, + unless the body mutates hash by inserting newer objects, + and also deleting them such that these variables end up + with the only reference. But in that case, those objects + will be noted in the GC's check list. */ *cdr_l(keyvar) = car(cell); *cdr_l(valvar) = cdr(cell); eval_progn(body, new_env, form); diff --git a/lib.c b/lib.c index 77231478..ae6fd998 100644 --- a/lib.c +++ b/lib.c @@ -3781,7 +3781,11 @@ static val sort_list(val list, val lessfun, val keyfun) return list; } else { val cons2 = cdr(list); - *cdr_l(cons2) = list; + /* This assignent is a dangerous mutation since the list + may contain mixtures of old and new objects, and + so we could be reversing a newer->older pointer + relationship. */ + set(*cdr_l(cons2), list); *cdr_l(list) = nil; return cons2; } @@ -3853,7 +3857,7 @@ val sort(val seq, val lessfun, val keyfun) objects. Sorting the list could reverse some of the pointers between the generations resulting in a backpointer. Thus we better inform the collector about this object. */ - return mut(sort_list(seq, lessfun, keyfun)); + return sort_list(seq, lessfun, keyfun); } sort_vec(seq, lessfun, keyfun); -- cgit v1.2.3