Skip to content

Commit d71d051

Browse files
author
Mark Pollack
committed
Add test to detect Mono.fromFuture() with JDK CompletableFutures
- New test noMonoFromFutureWithJdkCompletableFutures scans for Mono.fromFuture() usage - Excludes WebSocket transports (they configure HttpClient with custom executor) - Prevents regression of ForkJoinPool.commonPool pollution issue
1 parent b9090f9 commit d71d051

File tree

1 file changed

+69
-0
lines changed

1 file changed

+69
-0
lines changed

acp-core/src/test/java/com/agentclientprotocol/sdk/SchedulerBestPracticesTest.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,75 @@ void noImplicitSchedulerTimeOperators() throws IOException {
342342
.isEmpty();
343343
}
344344

345+
/**
346+
* Detects usage of {@code Mono.fromFuture()} which often wraps JDK CompletableFutures
347+
* that use ForkJoinPool.commonPool for completion.
348+
*
349+
* <p>Per BEST-PRACTICES-REACTIVE-SCHEDULERS.md, JDK APIs like {@code Process.onExit()},
350+
* {@code HttpClient} async methods, etc. use ForkJoinPool.commonPool for callbacks.
351+
* Wrapping these with Mono.fromFuture() doesn't change which thread runs the callbacks.
352+
*
353+
* <p>Alternatives:
354+
* <ul>
355+
* <li>Use blocking wait if in shutdown path: {@code process.waitFor()}</li>
356+
* <li>Add {@code .publishOn(scheduler)} after fromFuture to control downstream thread</li>
357+
* <li>Configure the underlying API with a custom executor (e.g., HttpClient.newBuilder().executor(...))</li>
358+
* </ul>
359+
*
360+
* <p>Note: WebSocket transports are excluded because they configure HttpClient with a custom
361+
* executor (acp-ws-client daemon threads), so their CompletableFutures don't use ForkJoinPool.
362+
*/
363+
@Test
364+
void noMonoFromFutureWithJdkCompletableFutures() throws IOException {
365+
List<String> violations = new ArrayList<>();
366+
367+
// Pattern to detect Mono.fromFuture() usage
368+
Pattern pattern = Pattern.compile("Mono\\.fromFuture\\(");
369+
370+
// Files that are known to use properly configured executors (not ForkJoinPool.commonPool)
371+
// WebSocket transports configure HttpClient with custom executor, so they're safe
372+
List<String> excludedFiles = List.of(
373+
"WebSocketAcpClientTransport.java",
374+
"WebSocketAcpAgentTransport.java"
375+
);
376+
377+
try (Stream<Path> paths = Files.walk(SOURCE_ROOT)) {
378+
paths.filter(Files::isRegularFile)
379+
.filter(p -> p.toString().endsWith(".java"))
380+
.filter(p -> excludedFiles.stream().noneMatch(exc -> p.toString().endsWith(exc)))
381+
.forEach(path -> {
382+
try {
383+
String content = Files.readString(path);
384+
String[] lines = content.split("\n");
385+
int lineNum = 0;
386+
for (String line : lines) {
387+
lineNum++;
388+
// Skip comment lines
389+
String trimmed = line.trim();
390+
if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) {
391+
continue;
392+
}
393+
Matcher matcher = pattern.matcher(line);
394+
if (matcher.find()) {
395+
violations.add(String.format(
396+
"%s:%d - Mono.fromFuture() may use ForkJoinPool.commonPool " +
397+
"(use blocking wait or add .publishOn(scheduler))",
398+
relativePath(path), lineNum));
399+
}
400+
}
401+
}
402+
catch (IOException e) {
403+
throw new RuntimeException("Failed to read: " + path, e);
404+
}
405+
});
406+
}
407+
408+
assertThat(violations)
409+
.describedAs("Mono.fromFuture() with JDK CompletableFutures uses ForkJoinPool.commonPool. " +
410+
"Use blocking wait in shutdown paths or add .publishOn(scheduler). Violations found")
411+
.isEmpty();
412+
}
413+
345414
/**
346415
* Gets the 1-based line number for a character position in the content.
347416
*/

0 commit comments

Comments
 (0)