mohamed-gallab-db opened a new pull request, #58079:
URL: https://github.com/apache/spark/pull/58079

   ### What changes were proposed in this pull request?
   
   `EchoProtocolSuite`'s `EngineClient` test helper touches its request 
`StreamObserver` from two threads without mutual exclusion. `sendCancel` does a 
check-then-act:
   
   - test thread: `if (requestCompleted.get()) return` reads `false`, then 
calls `requestObserver.onNext(Cancel)`;
   - meanwhile the response-observer callback thread runs 
`completeRequestStream()` (`requestObserver.onCompleted()`), half-closing the 
request stream.
   
   If the half-close lands between the `get()` and the `onNext`, the `onNext` 
throws, because gRPC does not permit concurrent calls on a `StreamObserver`.
   
   This adds a `requestLock` — mirroring the existing `responseLock`, which 
already serializes writes to the *response* observer — and guards both 
`sendCancel` (check + send) and `completeRequestStream` (CAS + half-close) so 
they can no longer interleave. Test-only change.
   
   ### Why are the changes needed?
   
   `EchoProtocolSuite`'s `cancel: engine sends Cancel after Finish` case sends 
a trailing `Cancel` exactly as the `FinishResponse` terminator arrives, so it 
hits the race above and intermittently fails with `IllegalStateException: call 
was half-closed` (or `Stream is already completed, no further calls are 
allowed`). It passes in isolation and only fails under scheduling load, so it 
can fail CI unpredictably.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   Existing `EchoProtocolSuite` cases. The race window is tiny, so to confirm 
the cause deterministically I temporarily widened it (a `Thread.sleep` between 
the `get()` and the `onNext` in `sendCancel`): with the widening the `cancel: 
engine sends Cancel after Finish` case failed on every run, and with 
`requestLock` in place it passed on every run with the same widening — 
confirming the lock closes the exact window. The temporary widening was then 
removed.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Anthropic)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to