imbajin commented on code in PR #2941:
URL:
https://github.com/apache/incubator-hugegraph/pull/2941#discussion_r2724264334
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/util/ConsumersTest.java:
##########
@@ -50,7 +50,35 @@ public void testStartProvideAwaitNormal() throws Throwable {
}
}
- @Test(timeout = 5000)
+ /**
+ * Regression test for deadlock:
+ *
+ * ContextCallable fails before entering runAndDone().
+ * await() must still return because latch is decremented in safeRun().
+ */
+ @Test(timeout = 1000)
+ public void testAwaitDoesNotHangWhenContextCallableFails() throws
Throwable {
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ try {
+ Consumers<Integer> consumers = new Consumers<>(executor, v -> {
+ // Never reached
+ });
+
+ /*
+ * start() creates ContextCallable internally.
+ * If ContextCallable.call() fails before runAndDone(),
+ * safeRun().finally MUST count down the latch.
+ */
+ consumers.start("test");
+
+ // If the fix is missing, this call hangs forever.
+ consumers.await();
+ } finally {
+ executor.shutdownNow();
+ }
+ }
Review Comment:
Seems current test cannot validate the failure(case)?
A possible solution may like:
```java
public void testAwaitDoesNotHangWhenFatalErrorOccurs() throws Throwable {
ExecutorService executor = Executors.newFixedThreadPool(1);
try {
// Use AssertionError to bypass the inner catch(Exception) loop
in runAndDone()
// This simulates a scenario where an exception escapes the task
logic
// (similar to how a ContextCallable failure would behave from
safeRun's perspective)
Consumers<Integer> consumers = new Consumers<>(executor, v -> {
throw new AssertionError("Simulated fatal error
(OOM/StackOverflow/etc)");
});
consumers.start("test-fatal-error");
consumers.provide(1);
// Verification:
// Without the fix, the latch would never be decremented
(because runAndDone crashes), causing await() to hang.
// With the fix (safeRun wrapper), the finally block ensures
latch.countDown() is called.
consumers.await();
// Note: consumer.exception will be null because safeRun only
catches Exception, not Error.
// This is acceptable behavior for fatal errors, as long as it
doesn't deadlock.
} finally {
executor.shutdownNow();
}
}
```
--
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]
