Skip to content

Commit 9dc5933

Browse files
edburnsCopilot
andcommitted
Prevent CopilotSession leak on assertion failure in TimeoutEdgeCaseTest
src/test/java/com/github/copilot/sdk/TimeoutEdgeCaseTest.java Wrap CopilotSession in try-with-resources in both tests so the session and its scheduler thread are always cleaned up, even if an assertion fails before the explicit close() call. In test 1, the explicit session.close() is kept because it is the action under test; the try-with-resources provides a safety net via idempotent double-close. In test 2, the explicit session.close() is removed since it was purely cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a01f0b5 commit 9dc5933

File tree

1 file changed

+29
-28
lines changed

1 file changed

+29
-28
lines changed

src/test/java/com/github/copilot/sdk/TimeoutEdgeCaseTest.java

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -74,19 +74,20 @@ public int read() throws IOException {
7474
void testTimeoutDoesNotFireAfterSessionClose() throws Exception {
7575
JsonRpcClient rpc = createHangingRpcClient();
7676
try {
77-
CopilotSession session = new CopilotSession("test-timeout-id", rpc);
77+
try (CopilotSession session = new CopilotSession("test-timeout-id", rpc)) {
7878

79-
CompletableFuture<AssistantMessageEvent> result = session
80-
.sendAndWait(new MessageOptions().setPrompt("hello"), 2000);
79+
CompletableFuture<AssistantMessageEvent> result = session
80+
.sendAndWait(new MessageOptions().setPrompt("hello"), 2000);
8181

82-
assertFalse(result.isDone(), "Future should be pending before timeout fires");
82+
assertFalse(result.isDone(), "Future should be pending before timeout fires");
8383

84-
// close() blocks up to 5s on session.destroy RPC. The 2s timeout
85-
// fires during that window with the current per-call scheduler.
86-
session.close();
84+
// close() blocks up to 5s on session.destroy RPC. The 2s timeout
85+
// fires during that window with the current per-call scheduler.
86+
session.close();
8787

88-
assertFalse(result.isDone(), "Future should not be completed by a timeout after session is closed. "
89-
+ "The per-call ScheduledExecutorService leaked a TimeoutException.");
88+
assertFalse(result.isDone(), "Future should not be completed by a timeout after session is closed. "
89+
+ "The per-call ScheduledExecutorService leaked a TimeoutException.");
90+
}
9091
} finally {
9192
rpc.close();
9293
}
@@ -105,31 +106,31 @@ void testTimeoutDoesNotFireAfterSessionClose() throws Exception {
105106
void testSendAndWaitReusesTimeoutThread() throws Exception {
106107
JsonRpcClient rpc = createHangingRpcClient();
107108
try {
108-
CopilotSession session = new CopilotSession("test-thread-count-id", rpc);
109+
try (CopilotSession session = new CopilotSession("test-thread-count-id", rpc)) {
109110

110-
long baselineCount = countTimeoutThreads();
111+
long baselineCount = countTimeoutThreads();
111112

112-
CompletableFuture<AssistantMessageEvent> result1 = session
113-
.sendAndWait(new MessageOptions().setPrompt("hello1"), 30000);
113+
CompletableFuture<AssistantMessageEvent> result1 = session
114+
.sendAndWait(new MessageOptions().setPrompt("hello1"), 30000);
114115

115-
Thread.sleep(100);
116-
long afterFirst = countTimeoutThreads();
117-
assertTrue(afterFirst >= baselineCount + 1,
118-
"Expected at least one new sendAndWait-timeout thread after first call. " + "Baseline: "
119-
+ baselineCount + ", after: " + afterFirst);
116+
Thread.sleep(100);
117+
long afterFirst = countTimeoutThreads();
118+
assertTrue(afterFirst >= baselineCount + 1,
119+
"Expected at least one new sendAndWait-timeout thread after first call. " + "Baseline: "
120+
+ baselineCount + ", after: " + afterFirst);
120121

121-
CompletableFuture<AssistantMessageEvent> result2 = session
122-
.sendAndWait(new MessageOptions().setPrompt("hello2"), 30000);
122+
CompletableFuture<AssistantMessageEvent> result2 = session
123+
.sendAndWait(new MessageOptions().setPrompt("hello2"), 30000);
123124

124-
Thread.sleep(100);
125-
long afterSecond = countTimeoutThreads();
126-
assertTrue(afterSecond == afterFirst,
127-
"Shared scheduler should reuse the same thread — no new threads after second call. "
128-
+ "After first: " + afterFirst + ", after second: " + afterSecond);
125+
Thread.sleep(100);
126+
long afterSecond = countTimeoutThreads();
127+
assertTrue(afterSecond == afterFirst,
128+
"Shared scheduler should reuse the same thread — no new threads after second call. "
129+
+ "After first: " + afterFirst + ", after second: " + afterSecond);
129130

130-
result1.cancel(true);
131-
result2.cancel(true);
132-
session.close();
131+
result1.cancel(true);
132+
result2.cancel(true);
133+
}
133134
} finally {
134135
rpc.close();
135136
}

0 commit comments

Comments
 (0)