Skip to content

Commit 8679cea

Browse files
stephanjclaude
andcommitted
fix(acp): prevent reader thread leak on transport close
AcpTransport.close() left the acp-reader thread blocked on readLine() when the child process had spawned grandchildren (e.g. `sh -c "... sleep 30"`) that inherited the stdout pipe. process.destroy() only terminates the direct child, so the pipe stayed open and the reader never reached EOF, causing testPing_timeout_returnsFalse to fail with a thread leak. Close process stdout explicitly to unblock readLine, interrupt the reader thread, and wait on destroyForcibly to ensure full cleanup. Also de-duplicate Xvfb startup in the Tests workflow: export DISPLAY via \$GITHUB_ENV so it persists to the Gradle step instead of starting Xvfb twice (which logged "Server is already active for display 99"). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0753a13 commit 8679cea

2 files changed

Lines changed: 14 additions & 4 deletions

File tree

.github/workflows/tests.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ jobs:
2828
run: sudo apt-get install -y xvfb
2929

3030
- name: Start Xvfb
31-
run: Xvfb :99 & export DISPLAY=:99
31+
run: |
32+
Xvfb :99 &
33+
echo "DISPLAY=:99" >> $GITHUB_ENV
3234
3335
- name: Run Gradle tests
3436
env:
@@ -40,9 +42,7 @@ jobs:
4042
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
4143
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
4244
AWS_REGION: ${{ secrets.AWS_REGION }}
43-
run: |
44-
Xvfb :99 & export DISPLAY=:99
45-
./gradlew test
45+
run: ./gradlew test
4646

4747
- name: Upload Test Reports
4848
if: always()

src/main/java/com/devoxx/genie/service/acp/protocol/AcpTransport.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,16 +262,26 @@ public void close() {
262262
pendingRequests.clear();
263263
if (process != null) {
264264
process.destroy();
265+
// Close stdout explicitly: the child may have spawned grandchildren that
266+
// inherit the pipe, leaving the reader thread blocked on readLine() even
267+
// after destroy(). Closing the stream unblocks it with an IOException.
268+
try {
269+
process.getInputStream().close();
270+
} catch (IOException ignored) {
271+
// best-effort
272+
}
265273
try {
266274
if (!process.waitFor(SHUTDOWN_WAIT_SECONDS, TimeUnit.SECONDS)) {
267275
process.destroyForcibly();
276+
process.waitFor(SHUTDOWN_WAIT_SECONDS, TimeUnit.SECONDS);
268277
}
269278
} catch (InterruptedException e) {
270279
Thread.currentThread().interrupt();
271280
process.destroyForcibly();
272281
}
273282
}
274283
if (readerThread != null) {
284+
readerThread.interrupt();
275285
try {
276286
readerThread.join(TimeUnit.SECONDS.toMillis(SHUTDOWN_WAIT_SECONDS));
277287
} catch (InterruptedException e) {

0 commit comments

Comments
 (0)