Commit d61c623e authored by aljee@hyper.cx's avatar aljee@hyper.cx Committed by ian@well-typed.com

Allow multiple C finalizers to be attached to a Weak#

The commit replaces mkWeakForeignEnv# with addCFinalizerToWeak#.
This new primop mutates an existing Weak# object and adds a new
C finalizer to it.

This change removes an invariant in MarkWeak.c, namely that the relative
order of Weak# objects in the list needs to be preserved across GC. This
makes it easier to split the list into per-generation structures.

The patch also removes a race condition between two threads calling
finalizeWeak# on the same WEAK object at that same time.
parent 5d9e686c
...@@ -1871,8 +1871,15 @@ primop MkWeakNoFinalizerOp "mkWeakNoFinalizer#" GenPrimOp ...@@ -1871,8 +1871,15 @@ primop MkWeakNoFinalizerOp "mkWeakNoFinalizer#" GenPrimOp
has_side_effects = True has_side_effects = True
out_of_line = True out_of_line = True
primop MkWeakForeignEnvOp "mkWeakForeignEnv#" GenPrimOp primop AddCFinalizerToWeakOp "addCFinalizerToWeak#" GenPrimOp
o -> b -> Addr# -> Addr# -> Int# -> Addr# -> State# RealWorld -> (# State# RealWorld, Weak# b #) Addr# -> Addr# -> Int# -> Addr# -> Weak# b
-> State# RealWorld -> (# State# RealWorld, Int# #)
{ {\tt addCFinalizerToWeak# fptr ptr flag eptr w} attaches a C
function pointer {\tt fptr} to a weak pointer {\tt w} as a finalizer. If
{\tt flag} is zero, {\tt fptr} will be called with one argument,
{\tt ptr}. Otherwise, it will be called with two arguments,
{\tt eptr} and {\tt ptr}. {\tt addCFinalizerToWeak#} returns
1 on success, or 0 if {\tt w} is already dead. }
with with
has_side_effects = True has_side_effects = True
out_of_line = True out_of_line = True
......
...@@ -191,17 +191,21 @@ typedef struct _StgStableName { ...@@ -191,17 +191,21 @@ typedef struct _StgStableName {
typedef struct _StgWeak { /* Weak v */ typedef struct _StgWeak { /* Weak v */
StgHeader header; StgHeader header;
StgClosure *cfinalizer; StgClosure *cfinalizers;
StgClosure *key; StgClosure *key;
StgClosure *value; /* v */ StgClosure *value; /* v */
StgClosure *finalizer; StgClosure *finalizer;
struct _StgWeak *link; struct _StgWeak *link;
} StgWeak; } StgWeak;
typedef struct _StgDeadWeak { /* Weak v */ typedef struct _StgCFinalizerList {
StgHeader header; StgHeader header;
struct _StgWeak *link; StgClosure *link;
} StgDeadWeak; void (*fptr)(void);
void *ptr;
void *eptr;
StgWord flag; /* has environment (0 or 1) */
} StgCFinalizerList;
/* Byte code objects. These are fixed size objects with pointers to /* Byte code objects. These are fixed size objects with pointers to
* four arrays, designed so that a BCO can be easily "re-linked" to * four arrays, designed so that a BCO can be easily "re-linked" to
......
...@@ -412,7 +412,7 @@ RTS_FUN_DECL(stg_threadStatuszh); ...@@ -412,7 +412,7 @@ RTS_FUN_DECL(stg_threadStatuszh);
RTS_FUN_DECL(stg_mkWeakzh); RTS_FUN_DECL(stg_mkWeakzh);
RTS_FUN_DECL(stg_mkWeakNoFinalizzerzh); RTS_FUN_DECL(stg_mkWeakNoFinalizzerzh);
RTS_FUN_DECL(stg_mkWeakForeignzh); RTS_FUN_DECL(stg_mkWeakForeignzh);
RTS_FUN_DECL(stg_mkWeakForeignEnvzh); RTS_FUN_DECL(stg_addCFinalizzerToWeakzh);
RTS_FUN_DECL(stg_finalizzeWeakzh); RTS_FUN_DECL(stg_finalizzeWeakzh);
RTS_FUN_DECL(stg_deRefWeakzh); RTS_FUN_DECL(stg_deRefWeakzh);
......
...@@ -321,7 +321,7 @@ typedef struct _RtsSymbolVal { ...@@ -321,7 +321,7 @@ typedef struct _RtsSymbolVal {
#define Maybe_Stable_Names SymI_HasProto(stg_mkWeakzh) \ #define Maybe_Stable_Names SymI_HasProto(stg_mkWeakzh) \
SymI_HasProto(stg_mkWeakNoFinalizzerzh) \ SymI_HasProto(stg_mkWeakNoFinalizzerzh) \
SymI_HasProto(stg_mkWeakForeignEnvzh) \ SymI_HasProto(stg_addCFinalizzerToWeakzh) \
SymI_HasProto(stg_makeStableNamezh) \ SymI_HasProto(stg_makeStableNamezh) \
SymI_HasProto(stg_finalizzeWeakzh) SymI_HasProto(stg_finalizzeWeakzh)
......
...@@ -374,14 +374,10 @@ stg_mkWeakzh ( gcptr key, ...@@ -374,14 +374,10 @@ stg_mkWeakzh ( gcptr key,
w = Hp - SIZEOF_StgWeak + WDS(1); w = Hp - SIZEOF_StgWeak + WDS(1);
SET_HDR(w, stg_WEAK_info, CCCS); SET_HDR(w, stg_WEAK_info, CCCS);
// We don't care about cfinalizer here. StgWeak_key(w) = key;
// Should StgWeak_cfinalizer(w) be stg_NO_FINALIZER_closure or StgWeak_value(w) = value;
// something else? StgWeak_finalizer(w) = finalizer;
StgWeak_cfinalizers(w) = stg_NO_FINALIZER_closure;
StgWeak_key(w) = key;
StgWeak_value(w) = value;
StgWeak_finalizer(w) = finalizer;
StgWeak_cfinalizer(w) = stg_NO_FINALIZER_closure;
ACQUIRE_LOCK(sm_mutex); ACQUIRE_LOCK(sm_mutex);
StgWeak_link(w) = W_[weak_ptr_list]; StgWeak_link(w) = W_[weak_ptr_list];
...@@ -398,61 +394,62 @@ stg_mkWeakNoFinalizzerzh ( gcptr key, gcptr value ) ...@@ -398,61 +394,62 @@ stg_mkWeakNoFinalizzerzh ( gcptr key, gcptr value )
jump stg_mkWeakzh (key, value, stg_NO_FINALIZER_closure); jump stg_mkWeakzh (key, value, stg_NO_FINALIZER_closure);
} }
stg_mkWeakForeignEnvzh ( gcptr key, STRING(stg_cfinalizer_msg,"Adding a finalizer to %p\n")
gcptr val,
W_ fptr, // finalizer stg_addCFinalizzerToWeakzh ( W_ fptr, // finalizer
W_ ptr, W_ ptr,
W_ flag, // has environment (0 or 1) W_ flag, // has environment (0 or 1)
W_ eptr ) W_ eptr,
gcptr w )
{ {
W_ payload_words, words; W_ c, info;
gcptr w, p;
ALLOC_PRIM (SIZEOF_StgWeak); ALLOC_PRIM (SIZEOF_StgCFinalizerList)
w = Hp - SIZEOF_StgWeak + WDS(1); c = Hp - SIZEOF_StgCFinalizerList + WDS(1);
SET_HDR(w, stg_WEAK_info, CCCS); SET_HDR(c, stg_C_FINALIZER_LIST_info, CCCS);
payload_words = 4; StgCFinalizerList_fptr(c) = fptr;
words = BYTES_TO_WDS(SIZEOF_StgArrWords) + payload_words; StgCFinalizerList_ptr(c) = ptr;
("ptr" p) = ccall allocate(MyCapability() "ptr", words); StgCFinalizerList_eptr(c) = eptr;
StgCFinalizerList_flag(c) = flag;
TICK_ALLOC_PRIM(SIZEOF_StgArrWords,WDS(payload_words),0); ("ptr" info) = ccall lockClosure(w "ptr");
SET_HDR(p, stg_ARR_WORDS_info, CCCS);
StgArrWords_bytes(p) = WDS(payload_words); if (info == stg_DEAD_WEAK_info) {
StgArrWords_payload(p,0) = fptr; // Already dead.
StgArrWords_payload(p,1) = ptr; unlockClosure(w, info);
StgArrWords_payload(p,2) = eptr; return (0);
StgArrWords_payload(p,3) = flag; }
// We don't care about the value here. StgCFinalizerList_link(c) = StgWeak_cfinalizers(w);
// Should StgWeak_value(w) be stg_NO_FINALIZER_closure or something else? StgWeak_cfinalizers(w) = c;
StgWeak_key(w) = key; unlockClosure(w, info);
StgWeak_value(w) = val;
StgWeak_finalizer(w) = stg_NO_FINALIZER_closure;
StgWeak_cfinalizer(w) = p;
ACQUIRE_LOCK(sm_mutex); recordMutable(w);
StgWeak_link(w) = W_[weak_ptr_list];
W_[weak_ptr_list] = w;
RELEASE_LOCK(sm_mutex);
IF_DEBUG(weak, ccall debugBelch(stg_weak_msg,w)); IF_DEBUG(weak, ccall debugBelch(stg_cfinalizer_msg,w));
return (w); return (1);
} }
stg_finalizzeWeakzh ( gcptr w ) stg_finalizzeWeakzh ( gcptr w )
{ {
gcptr f, arr; gcptr f, list;
W_ info;
("ptr" info) = ccall lockClosure(w "ptr");
// already dead? // already dead?
if (GET_INFO(w) == stg_DEAD_WEAK_info) { if (info == stg_DEAD_WEAK_info) {
unlockClosure(w, info);
return (0,stg_NO_FINALIZER_closure); return (0,stg_NO_FINALIZER_closure);
} }
f = StgWeak_finalizer(w);
list = StgWeak_cfinalizers(w);
// kill it // kill it
#ifdef PROFILING #ifdef PROFILING
// @LDV profiling // @LDV profiling
...@@ -469,19 +466,12 @@ stg_finalizzeWeakzh ( gcptr w ) ...@@ -469,19 +466,12 @@ stg_finalizzeWeakzh ( gcptr w )
// //
// Todo: maybe use SET_HDR() and remove LDV_recordCreate()? // Todo: maybe use SET_HDR() and remove LDV_recordCreate()?
// //
SET_INFO(w,stg_DEAD_WEAK_info); unlockClosure(w, stg_DEAD_WEAK_info);
LDV_RECORD_CREATE(w);
f = StgWeak_finalizer(w); LDV_RECORD_CREATE(w);
arr = StgWeak_cfinalizer(w);
StgDeadWeak_link(w) = StgWeak_link(w);
if (arr != stg_NO_FINALIZER_closure) { if (list != stg_NO_FINALIZER_closure) {
ccall runCFinalizer(StgArrWords_payload(arr,0), ccall runCFinalizers(list);
StgArrWords_payload(arr,1),
StgArrWords_payload(arr,2),
StgArrWords_payload(arr,3));
} }
/* return the finalizer */ /* return the finalizer */
...@@ -494,10 +484,21 @@ stg_finalizzeWeakzh ( gcptr w ) ...@@ -494,10 +484,21 @@ stg_finalizzeWeakzh ( gcptr w )
stg_deRefWeakzh ( gcptr w ) stg_deRefWeakzh ( gcptr w )
{ {
W_ code; W_ code, info;
gcptr val; gcptr val;
if (GET_INFO(w) == stg_WEAK_info) { info = GET_INFO(w);
if (info == stg_WHITEHOLE_info) {
// w is locked by another thread. Now it's not immediately clear if w is
// alive or not. We use lockClosure to wait for the info pointer to become
// something other than stg_WHITEHOLE_info.
("ptr" info) = ccall lockClosure(w "ptr");
unlockClosure(w, info);
}
if (info == stg_WEAK_info) {
code = 1; code = 1;
val = StgWeak_value(w); val = StgWeak_value(w);
} else { } else {
......
...@@ -438,6 +438,15 @@ INFO_TABLE(stg_WEAK,1,4,WEAK,"WEAK","WEAK") ...@@ -438,6 +438,15 @@ INFO_TABLE(stg_WEAK,1,4,WEAK,"WEAK","WEAK")
INFO_TABLE_CONSTR(stg_DEAD_WEAK,0,5,0,CONSTR,"DEAD_WEAK","DEAD_WEAK") INFO_TABLE_CONSTR(stg_DEAD_WEAK,0,5,0,CONSTR,"DEAD_WEAK","DEAD_WEAK")
{ foreign "C" barf("DEAD_WEAK object entered!") never returns; } { foreign "C" barf("DEAD_WEAK object entered!") never returns; }
/* ----------------------------------------------------------------------------
C finalizer lists
Singly linked lists that chain multiple C finalizers on a weak pointer.
------------------------------------------------------------------------- */
INFO_TABLE_CONSTR(stg_C_FINALIZER_LIST,1,4,0,CONSTR,"C_FINALIZER_LIST","C_FINALIZER_LIST")
{ foreign "C" barf("C_FINALIZER_LIST object entered!") never returns; }
/* ---------------------------------------------------------------------------- /* ----------------------------------------------------------------------------
NO_FINALIZER NO_FINALIZER
......
...@@ -16,18 +16,21 @@ ...@@ -16,18 +16,21 @@
#include "Prelude.h" #include "Prelude.h"
#include "Trace.h" #include "Trace.h"
// ForeignPtrs with C finalizers rely on weak pointers inside weak_ptr_list
// to always be in the same order.
StgWeak *weak_ptr_list; StgWeak *weak_ptr_list;
void void
runCFinalizer(void *fn, void *ptr, void *env, StgWord flag) runCFinalizers(StgCFinalizerList *list)
{ {
if (flag) StgCFinalizerList *head;
((void (*)(void *, void *))fn)(env, ptr); for (head = list;
else (StgClosure *)head != &stg_NO_FINALIZER_closure;
((void (*)(void *))fn)(ptr); head = (StgCFinalizerList *)head->link)
{
if (head->flag)
((void (*)(void *, void *))head->fptr)(head->eptr, head->ptr);
else
((void (*)(void *))head->fptr)(head->ptr);
}
} }
void void
...@@ -42,15 +45,7 @@ runAllCFinalizers(StgWeak *list) ...@@ -42,15 +45,7 @@ runAllCFinalizers(StgWeak *list)
} }
for (w = list; w; w = w->link) { for (w = list; w; w = w->link) {
StgArrWords *farr; runCFinalizers((StgCFinalizerList *)w->cfinalizers);
farr = (StgArrWords *)UNTAG_CLOSURE(w->cfinalizer);
if ((StgClosure *)farr != &stg_NO_FINALIZER_closure)
runCFinalizer((void *)farr->payload[0],
(void *)farr->payload[1],
(void *)farr->payload[2],
farr->payload[3]);
} }
if (task != NULL) { if (task != NULL) {
...@@ -91,8 +86,6 @@ scheduleFinalizers(Capability *cap, StgWeak *list) ...@@ -91,8 +86,6 @@ scheduleFinalizers(Capability *cap, StgWeak *list)
// count number of finalizers, and kill all the weak pointers first... // count number of finalizers, and kill all the weak pointers first...
n = 0; n = 0;
for (w = list; w; w = w->link) { for (w = list; w; w = w->link) {
StgArrWords *farr;
// Better not be a DEAD_WEAK at this stage; the garbage // Better not be a DEAD_WEAK at this stage; the garbage
// collector removes DEAD_WEAKs from the weak pointer list. // collector removes DEAD_WEAKs from the weak pointer list.
ASSERT(w->header.info != &stg_DEAD_WEAK_info); ASSERT(w->header.info != &stg_DEAD_WEAK_info);
...@@ -101,13 +94,7 @@ scheduleFinalizers(Capability *cap, StgWeak *list) ...@@ -101,13 +94,7 @@ scheduleFinalizers(Capability *cap, StgWeak *list)
n++; n++;
} }
farr = (StgArrWords *)UNTAG_CLOSURE(w->cfinalizer); runCFinalizers((StgCFinalizerList *)w->cfinalizers);
if ((StgClosure *)farr != &stg_NO_FINALIZER_closure)
runCFinalizer((void *)farr->payload[0],
(void *)farr->payload[1],
(void *)farr->payload[2],
farr->payload[3]);
#ifdef PROFILING #ifdef PROFILING
// A weak pointer is inherently used, so we do not need to call // A weak pointer is inherently used, so we do not need to call
......
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
extern rtsBool running_finalizers; extern rtsBool running_finalizers;
extern StgWeak * weak_ptr_list; extern StgWeak * weak_ptr_list;
void runCFinalizer(void *fn, void *ptr, void *env, StgWord flag); void runCFinalizers(StgCFinalizerList *list);
void runAllCFinalizers(StgWeak *w); void runAllCFinalizers(StgWeak *w);
void scheduleFinalizers(Capability *cap, StgWeak *w); void scheduleFinalizers(Capability *cap, StgWeak *w);
void markWeakList(void); void markWeakList(void);
......
...@@ -620,7 +620,7 @@ thread_obj (StgInfoTable *info, StgPtr p) ...@@ -620,7 +620,7 @@ thread_obj (StgInfoTable *info, StgPtr p)
case WEAK: case WEAK:
{ {
StgWeak *w = (StgWeak *)p; StgWeak *w = (StgWeak *)p;
thread(&w->cfinalizer); thread(&w->cfinalizers);
thread(&w->key); thread(&w->key);
thread(&w->value); thread(&w->value);
thread(&w->finalizer); thread(&w->finalizer);
......
...@@ -122,7 +122,7 @@ traverseWeakPtrList(void) ...@@ -122,7 +122,7 @@ traverseWeakPtrList(void)
* called on a live weak pointer object. Just remove it. * called on a live weak pointer object. Just remove it.
*/ */
if (w->header.info == &stg_DEAD_WEAK_info) { if (w->header.info == &stg_DEAD_WEAK_info) {
next_w = ((StgDeadWeak *)w)->link; next_w = w->link;
*last_w = next_w; *last_w = next_w;
continue; continue;
} }
...@@ -144,7 +144,6 @@ traverseWeakPtrList(void) ...@@ -144,7 +144,6 @@ traverseWeakPtrList(void)
next_w = w->link; next_w = w->link;
// and put it on the new weak ptr list. // and put it on the new weak ptr list.
// NB. we must retain the order of the weak_ptr_list (#7160)
if (weak_ptr_list == NULL) { if (weak_ptr_list == NULL) {
weak_ptr_list = w; weak_ptr_list = w;
} else { } else {
...@@ -332,7 +331,7 @@ markWeakPtrList ( void ) ...@@ -332,7 +331,7 @@ markWeakPtrList ( void )
evacuate((StgClosure **)last_w); evacuate((StgClosure **)last_w);
w = *last_w; w = *last_w;
if (w->header.info == &stg_DEAD_WEAK_info) { if (w->header.info == &stg_DEAD_WEAK_info) {
last_w = &(((StgDeadWeak*)w)->link); last_w = &(w->link);
} else { } else {
last_w = &(w->link); last_w = &(w->link);
} }
......
...@@ -469,10 +469,14 @@ wanteds = concat ...@@ -469,10 +469,14 @@ wanteds = concat
,closureField C "StgWeak" "key" ,closureField C "StgWeak" "key"
,closureField C "StgWeak" "value" ,closureField C "StgWeak" "value"
,closureField C "StgWeak" "finalizer" ,closureField C "StgWeak" "finalizer"
,closureField C "StgWeak" "cfinalizer" ,closureField C "StgWeak" "cfinalizers"
,closureSize C "StgDeadWeak" ,closureSize C "StgCFinalizerList"
,closureField C "StgDeadWeak" "link" ,closureField C "StgCFinalizerList" "link"
,closureField C "StgCFinalizerList" "fptr"
,closureField C "StgCFinalizerList" "ptr"
,closureField C "StgCFinalizerList" "eptr"
,closureField C "StgCFinalizerList" "flag"
,closureSize C "StgMVar" ,closureSize C "StgMVar"
,closureField C "StgMVar" "head" ,closureField C "StgMVar" "head"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment