From fcb43c633ec174fb0e44f2be193432e8c2f8b754 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 4 Aug 2026 13:17:47 +0100 Subject: [PATCH] Reactivate dead logcontexts We should still run the code in the logcontext even if it has finished. This is fine, though will log an error (like in the Python impl). The logcontext should not have finished if there is Rust work still ongoing. --- rust/src/deferred.rs | 40 +++---------------------------------- rust/src/logging/context.rs | 5 ----- 2 files changed, 3 insertions(+), 42 deletions(-) diff --git a/rust/src/deferred.rs b/rust/src/deferred.rs index 05d9e00912..082baa2429 100644 --- a/rust/src/deferred.rs +++ b/rust/src/deferred.rs @@ -18,7 +18,6 @@ use std::{ sync::{Arc, Mutex}, }; -use log::{debug, log_enabled, Level}; use once_cell::sync::OnceCell; use pyo3::{ create_exception, exceptions::PyException, exceptions::PyRuntimeError, intern, prelude::*, @@ -26,7 +25,7 @@ use pyo3::{ }; use tokio::sync::oneshot; -use crate::logging::context::{with_logcontext, DEBUG_LOGGER_NAME}; +use crate::logging::context::with_logcontext; use crate::tokio_runtime::runtime; create_exception!( @@ -242,42 +241,9 @@ where // Drive the awaitable in the captured logcontext. Restored here // as we're on the reactor thread (the only thread where the // context's `main_thread` check passes). - // - // Never re-start a context that has already finished. The - // request may have completed (or been cancelled; - // `create_deferred` does not propagate cancellation) while this - // task was still running. Restoring its context would log - // "Re-starting finished log context" and account our work - // against metrics that are already finalised, so such work runs - // in the sentinel instead. Both `__exit__` (which sets - // `finished`) and this check run on the reactor thread, so the - // check cannot race. let context = match &logcontext { - Some(handle) => { - let finished = handle - .logging_context() - .is_some_and(|ctx| ctx.borrow(py).is_finished()); - if finished { - if log_enabled!(target: DEBUG_LOGGER_NAME, Level::Debug) { - // Only a real context can be finished, so - // `logging_context()` is `Some` here. - if let Some(ctx) = handle.logging_context() { - debug!( - target: DEBUG_LOGGER_NAME, - "run_python_awaitable: captured logcontext {} has \ - finished; running in the sentinel", - ctx.bind(py).str()? - ); - } - } - None - } else { - handle.logging_context().map(|ctx| ctx.clone_ref(py)) - } - } - // Called from outside any scoped task: the sentinel. (The - // reactor thread is normally at the sentinel already, in which - // case the switch below is a no-op.) + Some(handle) => handle.logging_context().map(|ctx| ctx.clone_ref(py)), + // Called from outside any scoped task: the sentinel. None => None, }; diff --git a/rust/src/logging/context.rs b/rust/src/logging/context.rs index 10f5d69dd4..3f80d972ed 100644 --- a/rust/src/logging/context.rs +++ b/rust/src/logging/context.rs @@ -703,11 +703,6 @@ impl LoggingContext { } impl LoggingContext { - /// Whether `__exit__` has run. - pub(crate) fn is_finished(&self) -> bool { - self.finished - } - /// The context name as an owned Rust string. /// /// This copies the string data, so it is for error/debug paths only.