From 10533daea82f6ba7b424f77cc3a1454944e21a4f Mon Sep 17 00:00:00 2001 From: Clement Rey <cr.rey.clement@gmail.com> Date: Thu, 7 Nov 2024 15:02:50 +0100 Subject: [PATCH] do not ever drain ALL_RECORDINGS --- rerun_py/src/python_bridge.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/rerun_py/src/python_bridge.rs b/rerun_py/src/python_bridge.rs index 36a229af51b7..2d2e561e1429 100644 --- a/rerun_py/src/python_bridge.rs +++ b/rerun_py/src/python_bridge.rs @@ -314,9 +314,21 @@ fn shutdown(py: Python<'_>) { re_log::debug!("Shutting down the Rerun SDK"); // Release the GIL in case any flushing behavior needs to cleanup a python object. py.allow_threads(|| { - for (_, recording) in all_recordings().drain() { + // NOTE: Do **NOT** try and drain() `all_recordings` here. + // + // Doing so would drop the last remaining reference to these recordings, and therefore + // trigger their deallocation as well as the deallocation of all the Python and C++ data + // that they might transitively reference, but this is _NOT_ the right place to do so. + // This method is called automatically during shutdown via python's `atexit`, which is not + // a safepoint for deallocating these things, quite far from it. + // + // Calling `disconnect()` will already take care of flushing everything that can be flushed, + // and cleaning up everything that can be safely cleaned up, anyhow. + // Whatever's left can wait for the OS to clean it up. + for (_, recording) in all_recordings().iter() { recording.disconnect(); } + flush_garbage_queue(); }); }