Skip to content

Increment SESSION_COUNT when cloning a Session#8170

Closed
fuzyll wants to merge 1 commit into
devfrom
test_fix_8166
Closed

Increment SESSION_COUNT when cloning a Session#8170
fuzyll wants to merge 1 commit into
devfrom
test_fix_8166

Conversation

@fuzyll

@fuzyll fuzyll commented May 12, 2026

Copy link
Copy Markdown
Contributor

I believe this would fix #8166. It's pretty tricky to figure out what's going on, but the user gave me a minimal test case that looks something like:

let session = Session::new().expect("failed to initialize headless session");
{
    let session = session.clone();
    // ...
    let result = tokio::task::spawn_blocking(move || {
        let bv = session.load_with_progress(&input, |_, _| true)

I think what's happening is that the session.clone() is making a new Session while bypassing the registration so SESSION_COUNT is never actually incremented. Then, later, when another Session has Drop called on it, we call shutdown() prematurely and that ruins everything else.

It's not clear to me if this solution is the right way to go, or if we should instead just remove Clone from Session altogether, or if there's a third option that would be better. But, I believe this would at least fix the issue at hand.

@fuzyll fuzyll added this to the Krypton milestone May 12, 2026
@fuzyll fuzyll requested a review from emesare May 12, 2026 22:41
@fuzyll fuzyll self-assigned this May 12, 2026

@emesare emesare left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, in the future we may also want to make Session impl Copy with the same logic as Clone, that can be a discussion for later though

@fuzyll

fuzyll commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

This was rebased and merged as 3021a68. Closing.

@fuzyll fuzyll closed this May 13, 2026
@fuzyll fuzyll deleted the test_fix_8166 branch June 1, 2026 19:51
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.

Rust Session has improper Clone impl

2 participants