Skip to content

Commit

Permalink
get rid of the old all_recordings thing
Browse files Browse the repository at this point in the history
  • Loading branch information
teh-cmc committed Dec 22, 2023
1 parent 167b2a1 commit 29beb4b
Showing 1 changed file with 0 additions and 36 deletions.
36 changes: 0 additions & 36 deletions rerun_py/src/python_bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@ use re_ws_comms::RerunServerPort;
use once_cell::sync::{Lazy, OnceCell};
use parking_lot::Mutex;

// The bridge needs to have complete control over the lifetimes of the individual recordings,
// otherwise all the recording shutdown machinery (which includes deallocating C, Rust and Python
// data and joining a bunch of threads) can end up running at any time depending on what the
// Python GC is doing, which obviously leads to very bad things :tm:.
//
// TODO(#2116): drop unused recordings
fn all_recordings() -> parking_lot::MutexGuard<'static, HashMap<StoreId, RecordingStream>> {
static ALL_RECORDINGS: OnceCell<Mutex<HashMap<StoreId, RecordingStream>>> = OnceCell::new();
ALL_RECORDINGS.get_or_init(Default::default).lock()
}

type GarbageChunk = arrow2::chunk::Chunk<Box<dyn arrow2::array::Array>>;
type GarbageSender = crossbeam::channel::Sender<GarbageChunk>;
type GarbageReceiver = crossbeam::channel::Receiver<GarbageChunk>;
Expand Down Expand Up @@ -275,9 +264,6 @@ fn new_recording(
);
}

// NOTE: The Rust-side of the bindings must be in control of the lifetimes of the recordings!
all_recordings().insert(recording_id, recording.clone());

Ok(PyRecordingStream(recording))
}

Expand Down Expand Up @@ -331,22 +317,12 @@ fn new_blueprint(
);
}

// NOTE: The Rust-side of the bindings must be in control of the lifetimes of the recordings!
all_recordings().insert(blueprint_id, blueprint.clone());

Ok(PyRecordingStream(blueprint))
}

#[pyfunction]
fn shutdown() {
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() {
recording.disconnect();
}
flush_garbage_queue();
});
}

// --- Recordings ---
Expand Down Expand Up @@ -413,9 +389,6 @@ fn set_global_data_recording(
// to zero, which means dropping it, which means flushing it, which potentially means
// deallocating python-owned data, which means grabbing the GIL, thus we need to release the
// GIL first.
//
// NOTE: This cannot happen anymore with the new `ALL_RECORDINGS` thingy, but better safe than
// sorry.
py.allow_threads(|| {
let rec = RecordingStream::set_global(
rerun::StoreKind::Recording,
Expand Down Expand Up @@ -445,9 +418,6 @@ fn set_thread_local_data_recording(
// to zero, which means dropping it, which means flushing it, which potentially means
// deallocating python-owned data, which means grabbing the GIL, thus we need to release the
// GIL first.
//
// NOTE: This cannot happen anymore with the new `ALL_RECORDINGS` thingy, but better safe than
// sorry.
py.allow_threads(|| {
let rec = RecordingStream::set_thread_local(
rerun::StoreKind::Recording,
Expand Down Expand Up @@ -488,9 +458,6 @@ fn set_global_blueprint_recording(
// to zero, which means dropping it, which means flushing it, which potentially means
// deallocating python-owned blueprint, which means grabbing the GIL, thus we need to release the
// GIL first.
//
// NOTE: This cannot happen anymore with the new `ALL_RECORDINGS` thingy, but better safe than
// sorry.
py.allow_threads(|| {
let rec = RecordingStream::set_global(
rerun::StoreKind::Blueprint,
Expand Down Expand Up @@ -520,9 +487,6 @@ fn set_thread_local_blueprint_recording(
// to zero, which means dropping it, which means flushing it, which potentially means
// deallocating python-owned blueprint, which means grabbing the GIL, thus we need to release the
// GIL first.
//
// NOTE: This cannot happen anymore with the new `ALL_RECORDINGS` thingy, but better safe than
// sorry.
py.allow_threads(|| {
let rec = RecordingStream::set_thread_local(
rerun::StoreKind::Blueprint,
Expand Down

0 comments on commit 29beb4b

Please sign in to comment.