Skip to content

fix: queue sentinels in finally can hang append session during task cancellation - #129

Merged
quettabit merged 1 commit into
mainfrom
qb/118
Sep 12, 2026
Merged

fix: queue sentinels in finally can hang append session during task cancellation#129
quettabit merged 1 commit into
mainfrom
qb/118

Conversation

@quettabit

@quettabit quettabit commented Sep 11, 2026

Copy link
Copy Markdown
Member

closes #118

@quettabit
quettabit requested a review from a team as a code owner September 11, 2026 23:55
@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR prevents append-session cancellation from entering potentially blocking queue operations in finally blocks.

  • Sends input and acknowledgement sentinels only after normal producer completion.
  • Preserves sentinel delivery for both normal retry-loop exit paths.
  • Allows TaskGroup cancellation and failures to propagate without waiting for queue capacity.

Confidence Score: 5/5

The PR appears safe to merge; normal completion remains correctly signaled and abnormal termination now relies on TaskGroup cancellation rather than potentially blocking cleanup writes.

Both normal retry-loop exits enqueue the acknowledgement sentinel exactly once, while producer or retry failures cancel outstanding queue waits and propagate through the public append-session error handling.

Important Files Changed

Filename Overview
src/s2_sdk/_s2s/_append_session.py Moves queue sentinel writes out of cancellation-sensitive finally blocks while preserving normal append-session termination.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    I[Input producer] -->|append inputs| IQ[Input queue]
    IQ --> R[Retrying append task]
    R -->|append acknowledgements| AQ[Acknowledgement queue]
    AQ --> C[Generator consumer]
    I -->|normal completion: None| IQ
    R -->|normal completion: None| AQ
    E[Failure or cancellation] --> TG[TaskGroup cancellation]
    TG --> I
    TG --> R
    TG --> C
Loading

Reviews (1): Last reviewed commit: "initial commit" | Re-trigger Greptile

@quettabit
quettabit merged commit 564851e into main Sep 12, 2026
7 checks passed
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.

[Detail Bug] AppendSession: close() can hang indefinitely when connection stalls and input queue is full

1 participant