PDGGK commented on PR #10529:
URL: https://github.com/apache/seatunnel/pull/10529#issuecomment-4831746687

   Thanks @DanielLeens — I've folded in DanielCarter-stack's two requests on 
top of the original fix (commit pushed).
   
   `close()` now uses try-finally so `socket.close()` always runs even if 
`reader.close()` throws. I also added `SocketSourceReaderTest`, which starts a 
local `ServerSocket`, opens the reader once, and calls `pollNext()` twice — 
asserting the second call still reads its line instead of failing with "Socket 
is closed". I confirmed it the other way too: against the old per-`pollNext` 
try-with-resources the test fails with `SocketException: Socket is closed`, and 
it passes against the fix — so it genuinely guards the regression. Locally `mvn 
test -Dtest=SocketSourceReaderTest` is green.
   
   One detail for the record: the test uses `Boundedness.BOUNDED` so each 
`pollNext()` returns after one read chunk (under `UNBOUNDED` the read loop runs 
until EOF within a single `pollNext()`, which wouldn't let us observe per-call 
reuse), with a `CountDownLatch` keeping the two server writes separate so the 
assertions stay deterministic. No new dependency needed — JUnit 5 and Mockito 
are inherited from the root pom.
   
   The remaining gate looks like the maintainer-triggered Build/CI run. Thanks 
again for taking another look.
   


-- 
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]

Reply via email to