{{ message }}
Repository navigation
Conversation
Collaborator
The JSON-RPC SSE listeners guarded only onComplete() with a volatile completed flag, so onError, a parse failure, and post-cancellation signals could each still reach the error/completion consumer, delivering more than one terminal callback (including a null completion after an error or cancellation). Introduce one shared atomic transition. AbstractSSEEventListener now owns an AtomicBoolean and a signalTerminal(Throwable) helper that lets the first caller win via compareAndSet and delivers exactly one outcome (a non-null failure or a null normal completion); every later signal is dropped. onError, onComplete and the parse-error path all route through it. The 0.3 compatibility JSON-RPC listener, which does not share the base class, mirrors the same AtomicBoolean pattern. REST is left as is (both native and 0.3 use a no-op completion callback, an API decision). Tests cover complete-then-error, error-then-complete, repeated completion, a 32-thread concurrent race, and a final event followed by completion, asserting exactly one terminal callback in each. Signed-off-by: AlgoVoi <chopmob@gmail.com>
Signal normal completion before auto-closing streams for final events so cancellation cannot report a contradictory terminal outcome. Apply this to the shared listener and the v0.3 JSON-RPC listener. This fixes a2aproject#1170
kabir
force-pushed
the
fix/sse-listener-single-terminal-callback-1170
branch
from
September 24, 2026 16:37
54a25b1 to
29e414a
Compare
Collaborator
|
I have added a commit addressing the review comment |
Verify that an onError callback caused by closing a stream after a final event does not replace the normal completion outcome in native and v0.3 JSON-RPC listeners. This fixes a2aproject#1170
ehsavoie
approved these changes
Sep 24, 2026
Collaborator
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1170. The JSON-RPC SSE listeners guarded only
onComplete()with a volatilecompletedflag, soonError, a parse error, and post-cancellation signals could each still reach the error/completion consumer, delivering more than one terminal callback (including anullcompletion after an error or cancellation).Fix
Introduce one shared atomic transition.
AbstractSSEEventListenernow owns anAtomicBooleanand asignalTerminal(Throwable)helper: the first caller to wincompareAndSet(false, true)delivers exactly one outcome (a non-null failure or anullnormal completion) and every later signal is dropped.onError,onCompleteand the JSON-RPC error/parse path all route through it. The 0.3 compatibility JSON-RPC listener, which does not share the base class, mirrors the sameAtomicBooleanpattern. REST is left unchanged (both native and 0.3 use a no-op completion callback, an API decision as noted in the issue).Tests
Added coverage on both listeners for complete-then-error, error-then-complete, repeated completion, a 32-thread concurrent race, and a final event followed by completion, each asserting exactly one terminal callback.
mvn -pl client/transport/jsonrpc,compat-0.3/client/transport/jsonrpc -am testpasses (SSEEventListenerTest 13, SSEEventListener_v0_3_Test 12).Note
Scoped to the terminal-callback contract in the issue. A separate, related question: the listeners do not uniformly treat a malformed frame as terminal (the native
parseResponseEventcan raise an unchecked GsonJsonSyntaxExceptionout ofonMessage; the 0.3 listener logs and continues on a malformed frame). Whether a malformed frame should end the stream or be skipped is a semantics decision. Happy to follow up once you confirm the intended behaviour.