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 f020a1f52508ccdaebef759f8cfa086f160d4bbe..485c331a054164995a7b07fb9c9c3e9a7bad2adf 100644 (file)
--- 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) {