fix(global_cache): Py_IsFinalizing guards on dict-watcher + clear()

SonicField · SonicField · commit bf8982b14095 · 2026-04-22T09:13:41.000-07:00
ARM64 pydebug shutdown SEGV root-cause: GlobalCacheKey holds raw PyDictObject* pointers to globals/builtins. When globals is freed during Py_FinalizeEx (interpreter_clear → PyDict_Clear), the cache entry remains in map_ + watch_map_[builtins]. Subsequent PyDict_EVENT_CLEARED on builtins triggers updateCache, which derefs cache.key().globals → use-after-free. Pydebug poison fill (0xdd...) makes the UAF a hard SEGV; x86_64 release reads stale-but-readable type pointer and silently passes the JIT_CHECK. Two surgical guards address the FINALIZE-SPECIFIC manifestation: Part A — Python/jit_support/phoenix_init.cpp phoenix_dict_watcher early-returns when Py_IsFinalizing(). Cache state is about to be destroyed in phoenix_free; processing events on tearing-down dicts is moot. Pattern matches pyjit.cpp:1642 funcDestroyed precedent. Part B — Python/jit/global_cache.cpp GlobalCacheManager::clear() short-circuits when Py_IsFinalizing(). Avoids two failure modes: 1. Ci_Watchers_UnwatchDict (PyDict_Unwatch) reads dict internals on freed memory → UAF under pydebug 2. ci_watcher_state_fini ran first in phoenix_free → watcher_id is -1 → JIT_CHECK 'Invalid dict watcher ID -1' abort (observed in earlier gate logs, regression historically) SCOPE HONESTY: this is FINALIZE-SPECIFIC protection, NOT full root-cause for the raw-pointer-aliasing class. Mid-execution module unload (refcount→0 on globals while cache holds raw pointer) remains unaddressed. Filed as W27 GlobalCacheKey raw-pointer lifecycle re-architecture (docs/w27-globalcachekey-lifecycle.md, Tier 6+ scope). The 314d0f2 'weak reference semantics' comment was FALSIFIED — no mechanism actually provided weak-reference semantics; comments updated to name the actual mechanisms (Py_IsFinalizing guard, finalize-only protection). Diagnostic evidence (per Debug-First protocol): lldb backtrace on ARM64 pydebug push 59 falsifier: PyType_HasFeature(type=0xdddddddddddddddd) object.h:966 <-- SEGV hasOnlyUnicodeKeys() dict.h:52 GlobalCacheManager::updateCache() global_cache.cpp:325 GlobalCacheManager::notifyDictClear() global_cache.cpp:144 phoenix_dict_watcher() phoenix_init.cpp:58 PyDict_Clear → interpreter_clear → Py_FinalizeEx Test plan post-commit: testkeeper ARM64 pydebug --clean rebuild + falsifier rerun (test_phoenix_jit_inline_except_closure under timeout+redirect) + push 58 baseline regression check (8b7b935 still passes). testkeeper x86_64 compile-only PASS verified pre-commit at binary timestamp 1776874355 (chat L1872 build verification). Authorization chain: supervisor option (i) at chat L1862, theologian LEVEL-1 ratification at chat L1861. Pythia python#75 python#1 (trigger isolation) gates downstream emitCallExceptionHandler queue, NOT this fix.
diff --git a/Python/jit/global_cache.cpp b/Python/jit/global_cache.cpp
@@ -3,6 +3,7 @@
 #include "cinderx/Jit/global_cache.h"
 
 #include "cinderx/Common/dict.h"
+#include "cinderx/Common/py-portability.h"
 #include "cinderx/Common/util.h"
 #include "cinderx/Common/watchers_c.h"
 #include "cinderx/Jit/threaded_compile.h"
@@ -181,6 +182,28 @@ void GlobalCacheManager::notifyDictUnwatch(PyDictObject* dict) {
 }
 
 void GlobalCacheManager::clear() {
+  // During Py_FinalizeEx, watched dicts may already be torn down by
+  // interpreter_clear (PyDict_Clear on globals/builtins). Two
+  // failure modes if we proceed:
+  //   1. Ci_Watchers_UnwatchDict (PyDict_Unwatch) reads dict internals →
+  //      use-after-free on poison-filled freed memory under pydebug.
+  //   2. ci_watcher_state_fini ran first in phoenix_free, so the dict
+  //      watcher_id is -1 → PyDict_Unwatch returns -1 → JIT_CHECK abort
+  //      (observed historically in gate logs as 'Invalid dict watcher ID -1').
+  //
+  // Short-circuit: drop in-memory cache state without notifying anyone.
+  // The map_ + watch_map_ destruct naturally when ~GlobalCacheManager
+  // returns; Ref<PyUnicodeObject> in GlobalCacheKey targets immortal
+  // interned strings, so destruction is safe.
+  //
+  // NOTE: this is FINALIZE-SPECIFIC protection paired with the
+  // Py_IsFinalizing guard in phoenix_dict_watcher. Mid-execution module
+  // unload (cache holds raw ptr to a globals/builtins dict that gets
+  // freed) remains unaddressed — see W27 GlobalCacheKey raw-pointer
+  // lifecycle re-architecture.
+  if (Py_IsFinalizing()) {
+    return;
+  }
   std::vector<PyDictObject*> keys;
   for (auto& pair : watch_map_) {
     keys.push_back(pair.first);
diff --git a/Python/jit_support/phoenix_init.cpp b/Python/jit_support/phoenix_init.cpp
@@ -15,6 +15,7 @@
 #include "cinderx/Jit/frame.h"
 #include "cinderx/Common/code.h"
 #include "cinderx/Common/log.h"
+#include "cinderx/Common/py-portability.h"
 #include "cinderx/Common/watchers_c.h"
 #include "cinderx/Common/util.h"
 #include "cinderx/module_c_state.h"
@@ -30,6 +31,25 @@ static int phoenix_code_watcher(PyCodeEvent event, PyCodeObject* co) {
 static int phoenix_dict_watcher(
     PyDict_WatchEvent event, PyObject* dict_obj,
     PyObject* key_obj, PyObject* new_value) {
+    /* During Py_FinalizeEx, dicts are torn down (interpreter_clear,
+       module finalization). The GlobalCacheKey holds raw pointers to
+       globals + builtins dicts — once one of those is freed, processing
+       a CLEARED/MODIFIED event on the surviving companion would deref
+       the freed pointer (poisoned 0xdd... fill under pydebug → SEGV).
+
+       Skipping events during finalize is correct semantics: the cache
+       and its watch state are about to be destroyed in phoenix_free.
+       Same pattern as pyjit.cpp:1642 (funcDestroyed during finalize).
+
+       NOTE: this is FINALIZE-SPECIFIC protection, NOT a full root-cause
+       fix for the raw-pointer-aliasing class. Mid-execution module
+       unload (refcount→0 on globals while cache holds raw pointer)
+       remains unaddressed — filed as W27 GlobalCacheKey raw-pointer
+       lifecycle re-architecture. */
+    if (Py_IsFinalizing()) {
+        return 0;
+    }
+
     auto state = cinderx::getModuleState();
     jit::IGlobalCacheManager* globalCaches =
         state != nullptr ? state->cacheManager() : nullptr;
@@ -59,12 +79,13 @@ static int phoenix_dict_watcher(
             break;
         case PyDict_EVENT_CLONED:
         case PyDict_EVENT_DEALLOCATED:
-            /* Skip notifyDictUnwatch during deallocation/clone —
-               the dict may be partially freed during GC, and
-               notifyDictUnwatch accesses dict internals that can
-               trigger use-after-free. The GlobalCacheManager will
-               naturally stop watching when the dict pointer becomes
-               invalid (weak reference semantics). */
+            /* Skip notifyDictUnwatch during deallocation/clone outside
+               finalize — the dict may be partially freed during GC, and
+               notifyDictUnwatch could call Ci_Watchers_UnwatchDict on a
+               companion dict whose tp_dealloc is concurrent (the
+               2026-03-30 GC re-entrancy crash, fix 314d0f2310). The
+               finalize-specific class is now handled by the
+               Py_IsFinalizing early-return above. */
             break;
     }
     return 0;
diff --git a/docs/w27-globalcachekey-lifecycle.md b/docs/w27-globalcachekey-lifecycle.md
@@ -0,0 +1,87 @@
+# W27 — GlobalCacheKey Raw-Pointer Lifecycle Re-Architecture
+
+**Status:** Filed (generalist, 2026-04-22, supervisor-authorized [chat ~16:04Z])
+**Owner:** unassigned (Tier 6+ scope)
+**Schedule:** Post-Tier-5-close; not blocking any current work
+**Origin:** push 59 ARM64 pydebug SEGV root-cause (lldb @ global_cache.cpp:325)
+
+---
+
+## 1. Problem statement
+
+`jit::GlobalCacheKey` (Python/jit/global_cache.h) holds **raw `PyDictObject*` pointers** to a function's `globals` and `builtins` dicts:
+
+```cpp
+struct GlobalCacheKey {
+  PyDictObject* builtins;        // raw pointer
+  PyDictObject* globals;         // raw pointer
+  Ref<PyUnicodeObject> name;     // refcounted
+  ...
+};
+```
+
+These pointers can outlive the dicts they reference. `GlobalCacheManager` watches each dict via the dict-watcher API, but the **PyDict_EVENT_DEALLOCATED** handler is intentionally a no-op (commit 314d0f2310, March 2026) due to a 2026-03-30 GC re-entrancy crash in `notifyDictUnwatch`.
+
+Result: when `globals` is freed while `builtins` is still alive, the cache entry remains in `map_` with a stale `globals` pointer. Subsequent events on `builtins` (e.g. `notifyDictClear`) call `updateCache(cache, builtins, nullptr)`, which dereferences `cache.key().globals` → use-after-free.
+
+## 2. Observed manifestation
+
+- **Push 59** (b44f0752eb, 2026-04-22): falsifier `test_phoenix_jit_inline_except_closure` triggered ARM64 pydebug SEGV.
+- lldb backtrace: `phoenix_dict_watcher → notifyDictClear → updateCache → hasOnlyUnicodeKeys → PyType_HasFeature(0xdddd…)`.
+- Pattern: pydebug poison-fill on freed PyTypeObject reached via stale `globals` pointer.
+- x86_64-release silent because freed memory still contains stale-but-readable type pointer.
+
+## 3. Finalize-phase mitigation (LANDED, this commit)
+
+Two defensive guards address the **finalize-specific** manifestation:
+- `phoenix_dict_watcher`: early-return on `Py_IsFinalizing()` — skip processing all dict events during shutdown.
+- `GlobalCacheManager::clear()`: short-circuit on `Py_IsFinalizing()` — drop in-memory state without calling `Ci_Watchers_UnwatchDict` (watcher infra also torn down by then).
+
+These DO NOT address the deeper class.
+
+## 4. Unaddressed class
+
+**Mid-execution module unload**. If a Python module is unloaded (`del sys.modules[name]`, refcount on its `__dict__` reaches 0, dict deallocates) while the JIT cache holds a `GlobalCacheKey` keyed on that dict, the same UAF triggers — but `Py_IsFinalizing()` is `false`, so neither guard fires.
+
+Reproduction sketch (not yet tested):
+```python
+import sys, weakref, gc
+import some_jit_compiled_module as m
+m.compiled_function()  # registers global cache entry
+ref = weakref.ref(m.__dict__)
+del sys.modules['some_jit_compiled_module']
+del m
+gc.collect()
+assert ref() is None  # globals freed
+# Any subsequent event on builtins that touches the stale cache → UAF
+```
+
+## 5. Re-architecture options
+
+### Option A: Refcount the cached dicts
+Hold `Ref<PyDictObject>` instead of raw `PyDictObject*` in `GlobalCacheKey`. Pro: simplest. Con: extends dict lifetime, may interfere with module unload semantics (modules expected to be GC'd after `del sys.modules[name]`).
+
+### Option B: Weak references via dict watcher
+Implement true weak-reference semantics by handling `PyDict_EVENT_DEALLOCATED` correctly. Requires resolving the 2026-03-30 GC re-entrancy crash. Suggested approach: defer cache cleanup to a per-thread queue drained on the next safe point, rather than synchronous `notifyDictUnwatch` from inside the watcher callback.
+
+### Option C: Cache validation on read
+Mark cache entries invalid via a separate shadow map indexed by dict pointer; check validity at every cache read. Pro: lifecycle-independent. Con: per-read overhead.
+
+### Option D: Restructure cache key
+Drop `globals`/`builtins` raw pointers from the key entirely; key on the JIT'd function's identity, look up dicts dynamically. Pro: no aliasing issue. Con: changes hash semantics, cache invalidation gets complex.
+
+Likely best path: **Option B** with deferred-cleanup queue. Matches CPython's "watcher callback should not mutate watched state" guidance and resolves the 2026-03-30 re-entrancy.
+
+## 6. Sequencing
+
+- Pre-req: Tier 5 close (zero C++ in builder.cpp emit methods)
+- Falsifier needed before fix lands: mid-execution module-unload reproducer
+- Estimated scope: 3-4 hours for Option B, 1-2 hours for Option A (with caveats)
+- Risk: medium — touches global cache invariants, requires re-verification of all global-cache-related tests
+
+## 7. Cross-references
+
+- Push 59 root-cause analysis: chat 2026-04-22 ~15:55-16:04Z
+- Original DEALLOCATED-skip: 314d0f2310 (Alex, 2026-03-30)
+- Finalize-phase mitigation: this commit (paired with push 59)
+- Related lifecycle pattern: feedback_no_workarounds.md
