fix custodian-related memory-management problem

The problem mainly affected `register-custodian-shutdown`
from `ffi/unsafe/custodian`, which is used by `math/bigfloat`
and `ffi/unsafe/com`.

When a value is registered with a custodian, the value is held
weakly, but the shutdown procedure is intended to be held
strongly. At the C API level, the data associated with a shutdown
function pointer is intended to be held strongly.

A custodian itself, however, is retained weakly by other custodians
in its family, so that custodians can be GCed and their elements
transferred to a parent custodian. Since the custodian itself may
be held only weakly, the callback & data in a custodian was effectively
held weakly --- which, in turn, can break assumptions in code such
as `ffi/unsafe/custodian` that expects strong references to prevent
finalizers from running.

Fix the problem by registering a reference to callback data as
data in a custodian's finalizer, which makes the data strongly
retained no matter how the custodian is retained.
This commit is contained in:
Matthew Flatt 2013-10-11 12:31:32 -06:00
parent a22fd9e7ac
commit 7e42ee2003
5 changed files with 54 additions and 1 deletions

View File

@ -22,3 +22,41 @@
(unless done?
(error "shutdown didn't work"))
;; ----------------------------------------
;; Check that shutdown procedure is retained strongly
(define (go)
(define done? #f)
(define remembered (make-weak-hasheq))
(define r-count 0)
(define (remember v)
(hash-set! remembered v #t)
(set! r-count (add1 r-count))
v)
(define c (make-custodian))
(let ([c-sub (parameterize ([current-custodian c])
(make-custodian))]
[b (box #t)])
(parameterize ([current-custodian c-sub])
(void
(register-custodian-shutdown (box 0)
(remember
(lambda (x)
(set! done? (unbox b))))))))
(for ([i 2])
(collect-garbage))
(unless (= r-count (hash-count remembered))
(error 'remembered "gone! ~s" (hash-count remembered)))
(custodian-shutdown-all c)
done?)
(for ([i 10])
(unless (go)
(error "shutdown failed")))

View File

@ -106,6 +106,7 @@ static int mark_custodian_val_MARK(void *p, struct NewGC *gc) {
gcMARK2(m->mrefs, gc);
gcMARK2(m->closers, gc);
gcMARK2(m->data, gc);
gcMARK2(m->data_ptr, gc);
gcMARK2(m->parent, gc);
gcMARK2(m->sibling, gc);
@ -127,6 +128,7 @@ static int mark_custodian_val_FIXUP(void *p, struct NewGC *gc) {
gcFIXUP2(m->mrefs, gc);
gcFIXUP2(m->closers, gc);
gcFIXUP2(m->data, gc);
gcFIXUP2(m->data_ptr, gc);
gcFIXUP2(m->parent, gc);
gcFIXUP2(m->sibling, gc);

View File

@ -1894,6 +1894,7 @@ mark_custodian_val {
gcMARK2(m->mrefs, gc);
gcMARK2(m->closers, gc);
gcMARK2(m->data, gc);
gcMARK2(m->data_ptr, gc);
gcMARK2(m->parent, gc);
gcMARK2(m->sibling, gc);

View File

@ -665,6 +665,7 @@ struct Scheme_Custodian {
Scheme_Custodian_Reference **mrefs;
Scheme_Close_Custodian_Client **closers;
void **data;
void ***data_ptr; /* points to `data`, registered as finalizer data for strong retention */
/* weak indirections: */
Scheme_Custodian_Reference *parent;

View File

@ -922,6 +922,7 @@ static void ensure_custodian_space(Scheme_Custodian *m, int k)
m->boxes = naya_boxes;
m->closers = naya_closers;
m->data = naya_data;
*m->data_ptr = naya_data;
m->mrefs = naya_mrefs;
}
}
@ -1082,6 +1083,11 @@ static void adjust_custodian_family(void *mgr, void *skip_move)
CUSTODIAN_FAM(r->global_next) = NULL;
}
static void do_adjust_custodian_family(void *mgr, void *for_retain)
{
adjust_custodian_family(mgr, NULL);
}
void insert_custodian(Scheme_Custodian *m, Scheme_Custodian *parent)
{
/* insert into parent's list: */
@ -1118,6 +1124,7 @@ Scheme_Custodian *scheme_make_custodian(Scheme_Custodian *parent)
{
Scheme_Custodian *m;
Scheme_Custodian_Reference *mw;
void ***data_ptr;
if (!parent)
parent = main_custodian; /* still NULL if we're creating main; that's ok */
@ -1141,9 +1148,12 @@ Scheme_Custodian *scheme_make_custodian(Scheme_Custodian *parent)
CUSTODIAN_FAM(m->children) = NULL;
data_ptr = (void ***)scheme_malloc(sizeof(void**));
m->data_ptr = data_ptr;
insert_custodian(m, parent);
scheme_add_finalizer(m, adjust_custodian_family, NULL);
scheme_add_finalizer(m, do_adjust_custodian_family, data_ptr);
return m;
}
@ -1417,6 +1427,7 @@ Scheme_Thread *scheme_do_close_managed(Scheme_Custodian *m, Scheme_Exit_Closer_F
m->boxes = NULL;
m->closers = NULL;
m->data = NULL;
*m->data_ptr = NULL;
m->mrefs = NULL;
m->shut_down = 1;