Skip to content

Commit aeefebd

Browse files
committed
Defer journal sealing until callbacks stop
1 parent b58bc6c commit aeefebd

3 files changed

Lines changed: 58 additions & 23 deletions

File tree

SimpleAPI/src/main/java/com/bencodez/simpleapi/servercomm/http/HttpBackendTransportConnector.java

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,10 +343,25 @@ public boolean flushOutgoing(long deadlineNanos) {
343343
finishClose();
344344
}
345345
private void finishClose() {
346+
if (!shutdownCallbacks()) {
347+
// A callback may ignore interruption while still able to durably complete.
348+
// Keep journal ownership until it really terminates; sealing here would strand RUNNING.
349+
Thread reaper = new Thread(this::finishCloseAfterCallbacks, "SimpleAPI-HTTP-callback-reaper");
350+
reaper.setDaemon(true);
351+
reaper.start();
352+
return;
353+
}
354+
sealAfterCallbacks();
355+
}
356+
private void finishCloseAfterCallbacks() {
357+
boolean interrupted = false;
358+
while (!callbackExecutor.isTerminated()) try { callbackExecutor.awaitTermination(1, TimeUnit.DAYS); }
359+
catch (InterruptedException ignored) { interrupted = true; }
360+
if (interrupted) Thread.currentThread().interrupt();
361+
sealAfterCallbacks();
362+
}
363+
private void sealAfterCallbacks() {
346364
try {
347-
shutdownCallbacks();
348-
// No poller can enqueue more callbacks and every running journal transition has
349-
// finished, so ownership can now be revoked without stranding completed work.
350365
if (inboundDeliveries != null) inboundDeliveries.seal();
351366
if (acknowledgementConfirmationStore != null) acknowledgementConfirmationStore.seal();
352367
} finally { closed.countDown(); }
@@ -357,13 +372,13 @@ private void awaitClosed() {
357372
catch (InterruptedException stopRequested) { interrupted = true; }
358373
if (interrupted) Thread.currentThread().interrupt();
359374
}
360-
private void shutdownCallbacks() {
375+
private boolean shutdownCallbacks() {
361376
callbackExecutor.shutdown();
362377
boolean interrupted = false;
363378
try {
364379
if (!callbackExecutor.awaitTermination(5, TimeUnit.SECONDS)) {
365380
callbackExecutor.shutdownNow();
366-
callbackExecutor.awaitTermination(1, TimeUnit.SECONDS);
381+
return callbackExecutor.awaitTermination(1, TimeUnit.SECONDS);
367382
}
368383
} catch (InterruptedException stopRequested) {
369384
interrupted = true;
@@ -372,6 +387,7 @@ private void shutdownCallbacks() {
372387
catch (InterruptedException repeated) { interrupted = true; }
373388
}
374389
if (interrupted) Thread.currentThread().interrupt();
390+
return callbackExecutor.isTerminated();
375391
}
376392
boolean pollerAlive() { Thread current = poller; return current != null && current.isAlive(); }
377393
private static boolean joinPoller(Thread poller, long deadlineNanos) {

SimpleAPI/src/main/java/com/bencodez/simpleapi/servercomm/http/HttpProxyTransportServer.java

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -285,20 +285,32 @@ private boolean send(String serverId, String deliveryId, JsonEnvelope envelope,
285285
}
286286

287287
private void finishClose() {
288-
try {
289-
try { shutdown(handlerExecutor); }
290-
finally { shutdown(listenerExecutor); }
291-
} finally {
292-
try {
293-
synchronized (backends) {
294-
for (BackendState backend : backends.values()) { backend.seal(); backend.signal(); }
295-
backends.clear();
296-
}
297-
} finally { try { if (durableOutgoing != null) durableOutgoing.close(); }
298-
finally {
299-
synchronized (closeMonitor) { closeFinalizing = false; closeFinalized = true; closeMonitor.notifyAll(); }
300-
} }
288+
boolean handlersTerminated;
289+
try { handlersTerminated = shutdown(handlerExecutor); }
290+
finally { shutdown(listenerExecutor); }
291+
if (!handlersTerminated) {
292+
Thread reaper = new Thread(this::finishCloseAfterHandlers, "SimpleAPI-HTTP-proxy-handler-reaper");
293+
reaper.setDaemon(true);
294+
reaper.start();
295+
return;
301296
}
297+
sealAfterHandlers();
298+
}
299+
private void finishCloseAfterHandlers() {
300+
boolean interrupted = false;
301+
while (!handlerExecutor.isTerminated()) try { handlerExecutor.awaitTermination(1, TimeUnit.DAYS); }
302+
catch (InterruptedException ignored) { interrupted = true; }
303+
if (interrupted) Thread.currentThread().interrupt();
304+
sealAfterHandlers();
305+
}
306+
private void sealAfterHandlers() {
307+
try {
308+
synchronized (backends) {
309+
for (BackendState backend : backends.values()) { backend.seal(); backend.signal(); }
310+
backends.clear();
311+
}
312+
} finally { try { if (durableOutgoing != null) durableOutgoing.close(); }
313+
finally { synchronized (closeMonitor) { closeFinalizing = false; closeFinalized = true; closeMonitor.notifyAll(); } } }
302314
}
303315

304316
private void awaitClose() {
@@ -472,13 +484,13 @@ private static ThreadPoolExecutor executor(String name, int threads, int queue,
472484
return new ThreadPoolExecutor(threads, threads, 0L, TimeUnit.MILLISECONDS, new ArrayBlockingQueue<>(queue), factory, new ThreadPoolExecutor.AbortPolicy());
473485
}
474486
private static void setDefault(String name, String value) { if (System.getProperty(name) == null) System.setProperty(name, value); }
475-
static void shutdown(ExecutorService executor) {
487+
static boolean shutdown(ExecutorService executor) {
476488
executor.shutdown();
477489
boolean interrupted = false;
478490
try {
479491
if (!executor.awaitTermination(5, TimeUnit.SECONDS)) {
480492
executor.shutdownNow();
481-
executor.awaitTermination(1, TimeUnit.SECONDS);
493+
return executor.awaitTermination(1, TimeUnit.SECONDS);
482494
}
483495
} catch (InterruptedException stopRequested) {
484496
interrupted = true;
@@ -487,6 +499,7 @@ static void shutdown(ExecutorService executor) {
487499
catch (InterruptedException repeated) { interrupted = true; }
488500
}
489501
if (interrupted) Thread.currentThread().interrupt();
502+
return executor.isTerminated();
490503
}
491504

492505
public record ReceivedEnvelope(String serverId, String messageId, JsonEnvelope envelope) { }

SimpleAPI/src/test/java/com/bencodez/simpleapi/servercomm/http/HttpTransportRuntimeTest.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -721,9 +721,15 @@ void interruptedCloseRemainsBoundedWhenACallbackIgnoresInterruption() throws Exc
721721
release.countDown();
722722
assertTrue(stopped.await(2, TimeUnit.SECONDS));
723723
} finally { release.countDown(); }
724-
assertEquals(HttpInboundDeliveryStore.State.RUNNING,
725-
HttpInboundDeliveryStore.inspect(clientDirectory).state(id),
726-
"an ambiguous callback must remain fail-closed after bounded shutdown");
724+
long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(2);
725+
HttpInboundDeliveryStore.State state;
726+
do {
727+
state = HttpInboundDeliveryStore.inspect(clientDirectory).state(id);
728+
if (state == HttpInboundDeliveryStore.State.COMPLETED) break;
729+
Thread.sleep(10);
730+
} while (System.nanoTime() < deadline);
731+
assertEquals(HttpInboundDeliveryStore.State.COMPLETED, state,
732+
"the deferred journal seal must allow the late callback completion to become durable");
727733
}
728734

729735
@Test

0 commit comments

Comments
 (0)