From: Vincent Pit Date: Mon, 6 Apr 2015 18:45:46 +0000 (-0300) Subject: Reference-count global setup/teardown X-Git-Tag: v0.35~7 X-Git-Url: http://git.vpit.fr/?a=commitdiff_plain;h=b5ecb08dcda60cbbcb1c312717a1b5a4d2edb2ac;p=perl%2Fmodules%2Findirect.git Reference-count global setup/teardown This fixes surprises like this one : module loaded in thread 1, then in thread 2, then thread 1 ends and now the module does not work anymore in thread 2 (even though it should). --- diff --git a/indirect.xs b/indirect.xs index f020a1f..485c331 100644 --- a/indirect.xs +++ b/indirect.xs @@ -193,6 +193,8 @@ static void indirect_ck_restore(pTHX_ OPCODE type, indirect_ck_t *old_ck_p) { /* ... Check if the module is loaded ....................................... */ +static U32 indirect_loaded = 0; + #if I_THREADSAFE #define PTABLE_NAME ptable_loaded @@ -204,10 +206,7 @@ static void indirect_ck_restore(pTHX_ OPCODE type, indirect_ck_t *old_ck_p) { #define ptable_loaded_delete(T, K) ptable_loaded_delete(aPTBLMS_ (T), (K)) #define ptable_loaded_free(T) ptable_loaded_free(aPTBLMS_ (T)) -#define indirect_loaded() - -static ptable *indirect_loaded_cxts = NULL; -static U32 indirect_loaded_cxts_refcount = 0; +static ptable *indirect_loaded_cxts = NULL; /* We must use preexistent global mutexes or we will never be able to destroy * them. */ @@ -231,37 +230,48 @@ static int indirect_is_loaded(pTHX_ void *cxt) { return res; } -static void indirect_set_loaded(pTHX_ void *cxt) { -#define indirect_set_loaded(C) indirect_set_loaded(aTHX_ (C)) - I_LOADED_LOCK; - if (!indirect_loaded_cxts) { - indirect_loaded_cxts = ptable_new(); - indirect_loaded_cxts_refcount = 0; +static int indirect_set_loaded_locked(pTHX_ void *cxt) { +#define indirect_set_loaded_locked(C) indirect_set_loaded_locked(aTHX_ (C)) + int global_setup = 0; + + if (indirect_loaded <= 0) { + assert(!indirect_loaded_cxts); + indirect_loaded_cxts = ptable_new(); + global_setup = 1; } - ++indirect_loaded_cxts_refcount; + ++indirect_loaded; + assert(indirect_loaded_cxts); ptable_loaded_store(indirect_loaded_cxts, cxt, cxt); - I_LOADED_UNLOCK; + + return global_setup; } -static void indirect_clear_loaded(pTHX_ void *cxt) { -#define indirect_clear_loaded(C) indirect_clear_loaded(aTHX_ (C)) - I_LOADED_LOCK; - if (indirect_loaded_cxts_refcount <= 1) { - ptable_loaded_free(indirect_loaded_cxts); - indirect_loaded_cxts = NULL; - indirect_loaded_cxts_refcount = 0; - } else { - --indirect_loaded_cxts_refcount; +static int indirect_clear_loaded_locked(pTHX_ void *cxt) { +#define indirect_clear_loaded_locked(C) indirect_clear_loaded_locked(aTHX_ (C)) + int global_teardown = 0; + + if (indirect_loaded > 1) { + assert(indirect_loaded_cxts); ptable_loaded_delete(indirect_loaded_cxts, cxt); + --indirect_loaded; + } else if (indirect_loaded_cxts) { + ptable_loaded_free(indirect_loaded_cxts); + indirect_loaded_cxts = NULL; + indirect_loaded = 0; + global_teardown = 1; } - I_LOADED_UNLOCK; + + return global_teardown; } #else -#define indirect_is_loaded(C) (1) -#define indirect_set_loaded(C) NOOP -#define indirect_clear_loaded(C) NOOP +#define I_LOADED_LOCK NOOP +#define I_LOADED_UNLOCK NOOP + +#define indirect_is_loaded(C) (indirect_loaded > 0) +#define indirect_set_loaded_locked(C) ((indirect_loaded++ <= 0) ? 1 : 0) +#define indirect_clear_loaded_locked(C) ((--indirect_loaded <= 0) ? 1 : 0) #endif @@ -402,9 +412,11 @@ static void indirect_ptable_clone(pTHX_ ptable_ent *ent, void *ud_) { } static void indirect_thread_cleanup(pTHX_ void *ud) { + int global_teardown; dMY_CXT; - indirect_clear_loaded(&MY_CXT); + global_teardown = indirect_clear_loaded_locked(&MY_CXT); + assert(!global_teardown); SvREFCNT_dec(MY_CXT.global_code); MY_CXT.global_code = NULL; @@ -1019,77 +1031,31 @@ done: return o; } -/* --- Global setup/teardown ------------------------------------------------ */ - -static VOL U32 indirect_initialized = 0; +/* --- Module setup/teardown ----------------------------------------------- */ -static void indirect_global_teardown(pTHX_ void *root) { - if (!indirect_initialized) - return; +static void indirect_teardown(pTHX_ void *interp) { + dMY_CXT; #if I_MULTIPLICITY - if (aTHX != root) + if (aTHX != interp) return; #endif - indirect_ck_restore(OP_CONST, &indirect_old_ck_const); - indirect_ck_restore(OP_RV2SV, &indirect_old_ck_rv2sv); - indirect_ck_restore(OP_PADANY, &indirect_old_ck_padany); - indirect_ck_restore(OP_SCOPE, &indirect_old_ck_scope); - indirect_ck_restore(OP_LINESEQ, &indirect_old_ck_lineseq); - - indirect_ck_restore(OP_METHOD, &indirect_old_ck_method); - indirect_ck_restore(OP_METHOD_NAMED, &indirect_old_ck_method_named); - indirect_ck_restore(OP_ENTERSUB, &indirect_old_ck_entersub); - - indirect_initialized = 0; - - return; -} - -static void indirect_global_setup(pTHX) { -#define indirect_global_setup() indirect_global_setup(aTHX) - HV *stash; - - if (indirect_initialized) - return; - - PERL_HASH(indirect_hash, __PACKAGE__, __PACKAGE_LEN__); - - stash = gv_stashpvn(__PACKAGE__, __PACKAGE_LEN__, 1); - newCONSTSUB(stash, "I_THREADSAFE", newSVuv(I_THREADSAFE)); - newCONSTSUB(stash, "I_FORKSAFE", newSVuv(I_FORKSAFE)); - - indirect_ck_replace(OP_CONST, indirect_ck_const, &indirect_old_ck_const); - indirect_ck_replace(OP_RV2SV, indirect_ck_rv2sv, &indirect_old_ck_rv2sv); - indirect_ck_replace(OP_PADANY, indirect_ck_padany, &indirect_old_ck_padany); - indirect_ck_replace(OP_SCOPE, indirect_ck_scope, &indirect_old_ck_scope); - indirect_ck_replace(OP_LINESEQ, indirect_ck_scope, &indirect_old_ck_lineseq); - - indirect_ck_replace(OP_METHOD, indirect_ck_method, - &indirect_old_ck_method); - indirect_ck_replace(OP_METHOD_NAMED, indirect_ck_method_named, - &indirect_old_ck_method_named); - indirect_ck_replace(OP_ENTERSUB, indirect_ck_entersub, - &indirect_old_ck_entersub); - -#if I_MULTIPLICITY - call_atexit(indirect_global_teardown, aTHX); -#else - call_atexit(indirect_global_teardown, NULL); -#endif - - indirect_initialized = 1; - - return; -} + I_LOADED_LOCK; -/* --- Interpreter setup/teardown ------------------------------------------ */ + if (indirect_clear_loaded_locked(&MY_CXT)) { + indirect_ck_restore(OP_CONST, &indirect_old_ck_const); + indirect_ck_restore(OP_RV2SV, &indirect_old_ck_rv2sv); + indirect_ck_restore(OP_PADANY, &indirect_old_ck_padany); + indirect_ck_restore(OP_SCOPE, &indirect_old_ck_scope); + indirect_ck_restore(OP_LINESEQ, &indirect_old_ck_lineseq); -static void indirect_local_teardown(pTHX_ void *param) { - dMY_CXT; + indirect_ck_restore(OP_METHOD, &indirect_old_ck_method); + indirect_ck_restore(OP_METHOD_NAMED, &indirect_old_ck_method_named); + indirect_ck_restore(OP_ENTERSUB, &indirect_old_ck_entersub); + } - indirect_clear_loaded(&MY_CXT); + I_LOADED_UNLOCK; ptable_free(MY_CXT.map); MY_CXT.map = NULL; @@ -1102,21 +1068,52 @@ static void indirect_local_teardown(pTHX_ void *param) { return; } -static void indirect_local_setup(pTHX) { -#define indirect_local_setup() indirect_local_setup(aTHX) - MY_CXT_INIT; +static void indirect_setup(pTHX) { +#define indirect_setup() indirect_setup(aTHX) + MY_CXT_INIT; /* Takes/release PL_my_ctx_mutex */ + + I_LOADED_LOCK; + + if (indirect_set_loaded_locked(&MY_CXT)) { + PERL_HASH(indirect_hash, __PACKAGE__, __PACKAGE_LEN__); + + indirect_ck_replace(OP_CONST, indirect_ck_const, &indirect_old_ck_const); + indirect_ck_replace(OP_RV2SV, indirect_ck_rv2sv, &indirect_old_ck_rv2sv); + indirect_ck_replace(OP_PADANY, indirect_ck_padany, &indirect_old_ck_padany); + indirect_ck_replace(OP_SCOPE, indirect_ck_scope, &indirect_old_ck_scope); + indirect_ck_replace(OP_LINESEQ, indirect_ck_scope, &indirect_old_ck_lineseq); + + indirect_ck_replace(OP_METHOD, indirect_ck_method, + &indirect_old_ck_method); + indirect_ck_replace(OP_METHOD_NAMED, indirect_ck_method_named, + &indirect_old_ck_method_named); + indirect_ck_replace(OP_ENTERSUB, indirect_ck_entersub, + &indirect_old_ck_entersub); + } + + I_LOADED_UNLOCK; + + { + HV *stash; + + stash = gv_stashpvn(__PACKAGE__, __PACKAGE_LEN__, 1); + newCONSTSUB(stash, "I_THREADSAFE", newSVuv(I_THREADSAFE)); + newCONSTSUB(stash, "I_FORKSAFE", newSVuv(I_FORKSAFE)); #if I_THREADSAFE - MY_CXT.tbl = ptable_new(); - MY_CXT.owner = aTHX; + MY_CXT.tbl = ptable_new(); + MY_CXT.owner = aTHX; #endif - MY_CXT.map = ptable_new(); - MY_CXT.global_code = NULL; - - indirect_set_loaded(&MY_CXT); + MY_CXT.map = ptable_new(); + MY_CXT.global_code = NULL; + } - call_atexit(indirect_local_teardown, NULL); +#if I_MULTIPLICITY + call_atexit(indirect_teardown, aTHX); +#else + call_atexit(indirect_teardown, NULL); +#endif return; } @@ -1129,8 +1126,7 @@ PROTOTYPES: ENABLE BOOT: { - indirect_global_setup(); - indirect_local_setup(); + indirect_setup(); } #if I_THREADSAFE @@ -1158,7 +1154,13 @@ PPCODE: MY_CXT.tbl = t; MY_CXT.owner = aTHX; MY_CXT.global_code = global_code_dup; - indirect_set_loaded(&MY_CXT); + { + int global_setup; + I_LOADED_LOCK; + global_setup = indirect_set_loaded_locked(&MY_CXT); + assert(!global_setup); + I_LOADED_UNLOCK; + } } gv = gv_fetchpv(__PACKAGE__ "::_THREAD_CLEANUP", 0, SVt_PVCV); if (gv) {