Skip to content

fix: close all sessions in session_scope even when one close() raises (follow-up of #687) - #859

Closed
wdeveloper16 wants to merge 1 commit into
entrius:testfrom
wdeveloper16:fix/session-scope-leak
Closed

fix: close all sessions in session_scope even when one close() raises (follow-up of #687)#859
wdeveloper16 wants to merge 1 commit into
entrius:testfrom
wdeveloper16:fix/session-scope-leak

Conversation

@wdeveloper16

Copy link
Copy Markdown
Contributor

Follow-up to #687. Closes #858.

Problem

In session_scope()'s finally block, if the first session.close() raises, the loop aborts and every remaining session in the cache is leaked:

for s in cache.values():
    s.close()  # raises here → remaining sessions never closed

Fix

Wrap each close() individually so all sessions are always attempted. The first exception, if any, is re-raised after the full iteration:

first_exc: Optional[Exception] = None
for s in cache.values():
    try:
        s.close()
    except Exception as exc:
        if first_exc is None:
            first_exc = exc
if first_exc is not None:
    raise first_exc

Test

Adds test_all_sessions_closed_even_when_first_close_raises to TestSessionScope: two sessions are created, the first close() is made to raise, and the test asserts both sessions had close() called and _session_cache is None.

@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label Apr 29, 2026
@wdeveloper16
wdeveloper16 force-pushed the fix/session-scope-leak branch 7 times, most recently from e3c59a5 to 368354b Compare May 1, 2026 18:52
@wdeveloper16

Copy link
Copy Markdown
Contributor Author

Hi, @anderdc
I'd appreciate your feedback with this.

@wdeveloper16
wdeveloper16 force-pushed the fix/session-scope-leak branch 2 times, most recently from 5b124a2 to 69f5587 Compare May 2, 2026 19:10
@wdeveloper16

Copy link
Copy Markdown
Contributor Author

Hi, @anderdc
Could you please review the PR and share me your feedback?

@wdeveloper16
wdeveloper16 force-pushed the fix/session-scope-leak branch from 0dee529 to 7101520 Compare May 4, 2026 21:10
@anderdc

anderdc commented May 5, 2026

Copy link
Copy Markdown
Collaborator

requests.Session.close() doesn't raise in the standard urllib3 stack. On the off chance it did, _session_cache = None drops the references right before the loop and GC reclaims the rest. Defensive guard for a scenario the underlying library doesn't produce. Closing.

@anderdc anderdc closed this May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: session_scope leaks remaining sessions when close() raises

2 participants