Skip to content

Commit 23a73cd

Browse files
authored
fix: deliver mid-action cancels through SessionCancelHandle (#312)
cancel_action previously failed with 'session is busy' whenever an action was running, because Session::cancel_action needs &mut Session and a background action thread owns the Session for exactly that window - the primary window in which callers want to cancel. openjd-sessions 0.4.0 introduced Session::cancel_handle(), a thread-safe handle that delivers a cancel to the in-flight action without touching the Session, following the action's declared cancelation method on both the direct and cross-user helper paths. Obtain the handle at construction and route the taken-session case through it. The handle cannot perform the Running -> Canceling state transition (it does not own the Session), so the snapshot is updated to Canceling directly, keeping the state visible to Python-side pollers until the terminal state lands. Signed-off-by: Sean Tang <171081544+seant-aws@users.noreply.github.com>
1 parent a2cc32f commit 23a73cd

1 file changed

Lines changed: 35 additions & 11 deletions

File tree

rust-bindings/src/sessions/session.rs

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,11 @@ pub(crate) struct PySession {
265265
session: Arc<Mutex<Option<Session>>>,
266266
/// Snapshot updated after each state change, readable without blocking on the session.
267267
snapshot: Arc<Mutex<StateSnapshot>>,
268+
/// Thread-safe cancel handle, obtained at construction. Delivers a cancel
269+
/// to whichever action is running even while the `Session` value is owned
270+
/// by a background action thread — the case where `Session::cancel_action`
271+
/// is unreachable (it needs `&mut Session`).
272+
cancel_handle: openjd_sessions::session::SessionCancelHandle,
268273
}
269274

270275
impl PySession {
@@ -360,9 +365,11 @@ impl PySession {
360365
snap.files_directory = session.files_directory().to_string_lossy().to_string();
361366
snap.action_status = session.action_status();
362367
}
368+
let cancel_handle = session.cancel_handle();
363369
Ok(PySession {
364370
session: Arc::new(Mutex::new(Some(session))),
365371
snapshot,
372+
cancel_handle,
366373
})
367374
}
368375

@@ -643,19 +650,36 @@ impl PySession {
643650
time_limit: Option<f64>,
644651
mark_action_failed: Option<bool>,
645652
) -> PyResult<()> {
646-
// cancel_action can be called while an action is running (session is taken).
647-
// The Rust Session supports this via CancellationToken which is checked by the
648-
// subprocess runner. But we don't have access to &mut Session here.
649-
// For now, this is a limitation — cancel requires the session to not be taken.
650653
let duration = time_limit.map(std::time::Duration::from_secs_f64);
654+
let mark_failed = mark_action_failed.unwrap_or(false);
655+
// Direct path when the session is not taken (e.g. between actions):
656+
// Session::cancel_action also performs the Running -> Canceling state
657+
// transition, which the handle cannot (it doesn't own the Session).
651658
let mut guard = lock_recover(&self.session);
652-
match guard.as_mut() {
653-
Some(session) => session
654-
.cancel_action(duration, mark_action_failed.unwrap_or(false))
655-
.map_err(session_err_to_py),
656-
None => Err(pyo3::exceptions::PyRuntimeError::new_err(
657-
"Cannot cancel: session is busy with an action",
658-
)),
659+
if let Some(session) = guard.as_mut() {
660+
let result = session
661+
.cancel_action(duration, mark_failed)
662+
.map_err(session_err_to_py);
663+
Self::update_snapshot(session, &self.snapshot);
664+
return result;
665+
}
666+
drop(guard);
667+
668+
// Session is owned by a background action thread: deliver through the
669+
// thread-safe cancel handle. Cancellation follows the action's own
670+
// cancelation method (including cross-user helper delivery), exactly
671+
// like Session::cancel_action.
672+
if self.cancel_handle.cancel(duration, mark_failed) {
673+
// The handle cannot set the session's transient Canceling state;
674+
// reflect it in the snapshot so Python-side pollers observe the
675+
// cancel-in-progress phase until the terminal state lands.
676+
let mut snap = lock_recover(&self.snapshot);
677+
snap.state = SessionState::Canceling;
678+
Ok(())
679+
} else {
680+
Err(pyo3::exceptions::PyRuntimeError::new_err(
681+
"Cannot cancel: no action is running",
682+
))
659683
}
660684
}
661685

0 commit comments

Comments
 (0)