fix(GADP-051): bound GossClient receive with a timeout to fix cold-start hang - #50
Merged
craigpnnl merged 2 commits intoAug 1, 2026
Conversation
MessageConsumer.receive() with no timeout blocks the calling thread indefinitely waiting for a reply. A caller polling a service that has not started answering yet (a boot-order race, e.g. the topology background service at cold start) could hang forever on the very first attempt and never get the chance to retry. Add a bounded-timeout overload (receive(timeoutMillis)) alongside the existing unbounded getResponse() methods rather than changing their signature: existing callers keep their exact prior block-indefinitely behavior (receive(0) is documented to mean the same thing as the no-arg receive() it replaces), and new callers that need a retry loop can opt into a bounded wait. A timed-out receive returns null cleanly and still closes the JMS consumer; no exception leaks to the caller.
…uard oversized bodies - Add GossClientBoundedReceiveWallClockTest: prove bounded-receive against a real embedded ActiveMQ broker's wall-clock blocking behavior, not only a mocked MessageConsumer. - Add GossClientSyncReceiveWithActiveListenerTest: cover the session-isolation fix where a synchronous getResponse() must not run on a Session carrying an async MessageListener; derive a dedicated listener-free Session per call instead. - Guard BytesMessage reply bodies with an 8MiB cap (MAX_BYTES_MESSAGE_BODY_LENGTH) to reject an unbounded wire-supplied length before allocating a byte[] (OOM/DoS vector, and the getBodyLength() (int) cast is lossy past Integer.MAX_VALUE). - Add activemq-broker test-scope dependency (build.gradle) for the wall-clock test. - Extend GossClientBoundedReceiveTest and add javadoc for the timeout semantics of the bounded getResponse overload.
4 tasks
craigpnnl
added a commit
that referenced
this pull request
Aug 1, 2026
./gradlew spotlessCheck --no-daemon || true in the spotless job made the 'Code Formatting Check' check permanently green regardless of outcome, letting a real formatting violation merge on PR #50 while the separate honest gate in format-check.yml reported the failure correctly. Remove the || true so this job fails honestly on a real formatting violation. The quality-gate job already excludes spotless.result from its pass/fail decision (it only hard-fails on dependency-check), so this does not turn spotless into an unintended new blocker on the overall pipeline; it only fixes the per-check status shown on the PR.
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
1 participant
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.
Problem
During GridAPPS-D cold start, the FieldBusManager's topology poll can fire
before the Python topology processor has finished subscribing and is able
to answer. GossClient.getResponse() calls receive() with no timeout, so
when the reply never arrives (because the answering side was not yet up),
the call blocks indefinitely instead of failing fast so the caller can
retry.
Fix
Bound GossClient.getResponse()'s receive() call with a configurable
timeout instead of blocking forever. On timeout, getResponse() returns
control to the caller (rather than hanging), which lets a retry loop in
the caller (see the companion GOSS-GridAPPS-D PR) recover from a cold
start where the answering side is not yet ready.
Also adds test coverage for ObjectMessage/BytesMessage reply dispatch and
a guard against oversized message bodies.
Test results
36/36 tests passing in this repo.
Review
cold-start reproduction against this fix and confirmed the timeout/retry
path resolves the hang. Passed.
Companion PR
This fix is a prerequisite for the retry loop added in
GRIDAPPSD/GOSS-GridAPPS-D. That PR depends on this GossClient timeout
change landing first: GRIDAPPSD/GOSS-GridAPPS-D#1898