Skip to content

Add a drop safe async version of scope guard - #5910

Open
jsdt wants to merge 1 commit into
alessandro/refuse-takeover-for-same-session-and-teardownfrom
jsdt/async-scope-guard
Open

Add a drop safe async version of scope guard#5910
jsdt wants to merge 1 commit into
alessandro/refuse-takeover-for-same-session-and-teardownfrom
jsdt/async-scope-guard

Conversation

@jsdt

@jsdt jsdt commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Description of Changes

This is a change to make sure our client cleanup is drop safe. Our previous code had:

    let mut client = scopeguard::guard((client, session), |(client, session)| {
        tokio::spawn(ws_client_teardown(client, session));
    });

    ws_client_actor_inner(&mut client.0, options, ws, sendrx).await;

    let (client, session) = ScopeGuard::into_inner(client);
    ws_client_teardown(client, session).await;

This has an race condition, because if the client task were cancelled immediately after ws_client_actor_inner(&mut client.0, options, ws, sendrx).await;, then we would stop running ws_client_teardown on the first deferred task. To make sure we always run that cleanup to completion, we need to run it inside a tokio::spawn. We could fix it with a spawn, but I added a helper that works like scopeguard::guard for async cleanup functions. I'm actually surprised that we don't seem to do this pattern anywhere else.

Rollback safety impact

None.

Expected complexity level and risk

Testing

This has some unit tests for the new guard.

@jsdt
jsdt requested a review from aasoni September 9, 2026 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants