Release Module native caches on device close to fix nightly LSan leak - #1131
Release Module native caches on device close to fix nightly LSan leak#1131jkiviluoto-nv wants to merge 1 commit into
Conversation
NativeCallDataCache/NativeBoundCallRuntime derive from sgl::Object, which uses plain Py_INCREF/Py_DECREF once owned by Python with no tp_traverse/tp_clear. A Module that stays alive past a test (e.g. via LOADED_MODULES or a retained traceback) keeps its whole call_data_cache tree reachable until process exit, which LeakSanitizer reports as a direct leak in NativeBoundCallRuntime::set_args even though it is just an unreleased cache, not a true leak. Device::close() is documented to remove cyclic references that would otherwise prevent cleanup, and is invoked for every open device by the atexit-registered close_all_devices() call - but it has no visibility into the Python-level Module wrapper or its caches. Reuse the same device-close-callback mechanism already used for hot reload to release each Module's call_data_cache and related caches when its device closes, mirroring the reset already done in on_hot_reload. Fixes shader-slang#1130
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The Windows Release build failure ( |
2 participants
Summary
Fixes the deterministic LeakSanitizer finding reported in #1130: every scheduled
sanitizers.ymlrun on the Linuxasan-ubsanleg has failed since the workflow was introduced, on aNativeBoundCallRuntime::argssetter allocation (std::vector<sgl::ref<NativeBoundVariableRuntime>>).Root cause
NativeCallDataCache,NativeCallData,NativeBoundCallRuntime, andNativeBoundVariableRuntimeall derive fromsgl::Object, which is bound to Python via nanobind'snb::intrusive_ptrmechanism (src/slangpy_ext/core/object.cpp). That mechanism uses plainPy_INCREF/Py_DECREFonce an instance is owned by Python — there is notp_traverse/tp_clear, so these objects are invisible to Python's cyclic GC.slangpy.core.module.Module(pure Python) ownscall_data_cache(aCallDataCache/NativeCallDataCachesubclass) and a strong_attr_cache: dict[str, Function]. Calling aFunctionpopulatescall_data_cachewith aNativeCallData, which holdsruntime()— aNativeBoundCallRuntimewhosem_args/m_kwargsare exactly the leakedNativeBoundVariableRuntimetrees.Nothing currently ties a
Module's lifetime, or itscall_data_cache, toDevice.close().Device::close()is documented to "remove all cyclic references that might prevent the device from being destroyed," and is invoked for every open device via theatexit-registeredDevice::close_all_devices()(src/slangpy_ext/slangpy_ext.cpp). But that path is pure C++ and has no visibility into the Python-levelModulewrapper or its caches. If anyModulestays alive until interpreter shutdown (e.g. referenced from the module-levelLOADED_MODULESweak dict combined with some other live reference, or a retained pytest traceback), its wholecall_data_cachetree stays reachable-but-unreleased at process exit — which LSan reports as a leak even though it's just an unreleased cache, not a genuine leak.Fix
Modulealready has a proven pattern for releasing these caches:on_hot_reload()replacescall_data_cachewith a freshCallDataCache()and clears_attr_cache,dispatch_data_cache,pipeline_cache, andshader_table_cache. This PR extracts that intoModule._release_native_caches()and also calls it from a newdevice.register_device_close_callbackhook, registered alongside the existing hot-reload hook in_register_hot_reload_hook. Device-close callbacks run "at start of device close," before the device tears down, so replacing these caches is safe (nothing is dereferenced, only re-pointed at empty containers).This means every path that closes a device — explicit
device.close()and theatexit-drivenclose_all_devices()alike — now also releases every liveModule's native cache tree, matchingDevice::close()'s documented purpose.Verification
I was unable to get a working ASan+UBSan LeakSanitizer repro locally (clang-14's LD_PRELOAD combination for ASan+UBSan is broken via a
sigactioninterceptor bug, clang-18's apt.llvm.org build has an internal UBSan ABI mismatch, and clang-19 built and linked cleanly butimport slangpyspun in ananothread/ASan interceptor-relatedsched_yieldloop that looked unrelated to this leak). This fix is therefore based on static analysis of the reference-counting/ownership chain, not a live before/after LSan comparison — thesanitizers.ymlnightly run is the actual verification.