diff --git a/.gitignore b/.gitignore index 5581f51dc1..6534e4a9bc 100644 --- a/.gitignore +++ b/.gitignore @@ -52,6 +52,7 @@ local.properties *.sh !.evergreen/*.sh !scripts/*.sh +!testing/java-6279-poc/*.sh # security-sensitive files *.gpg diff --git a/driver-core/src/main/com/mongodb/internal/connection/PowerOfTwoBufferPool.java b/driver-core/src/main/com/mongodb/internal/connection/PowerOfTwoBufferPool.java index a8c7f87a24..26ddd595de 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/PowerOfTwoBufferPool.java +++ b/driver-core/src/main/com/mongodb/internal/connection/PowerOfTwoBufferPool.java @@ -28,9 +28,10 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.ConcurrentLinkedDeque; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; /** *
This class is not part of the public API and may be removed or changed at any time
@@ -40,6 +41,13 @@ public class PowerOfTwoBufferPool implements BufferProvider { /** * The global default pool. Pruning is enabled on this pool. Idle buffers are pruned after one minute. + * + *The pruner thread does not run all the time. It starts when the pool holds a buffer. It stops when the pool + * becomes empty.
+ * + *The pruner thread must stop. A thread that runs forever keeps the class loader of all driver classes in + * memory. The static data of those classes also stays in memory. Then an application server cannot unload the + * application. See JAVA-6279.
*/ public static final PowerOfTwoBufferPool DEFAULT = new PowerOfTwoBufferPool().enablePruning(); @@ -63,7 +71,13 @@ public ByteBuffer getBuffer() { private final MapThis method does not start a thread. An empty pool has no buffers to prune. The pruner starts when you + * {@linkplain #release(ByteBuffer) release} a buffer. The pruner stops when the pool becomes empty.
*/ PowerOfTwoBufferPool enablePruning() { - pruner.scheduleAtFixedRate(this::prune, maxIdleTimeNanos, maxIdleTimeNanos / 2, TimeUnit.NANOSECONDS); + pruningEnabled = true; + if (!allPoolsEmpty()) { + // The pool can hold buffers from before this call, and those buffers also need a prune. An empty pool + // must not start a thread. + startPruningIfNeeded(); + } return this; } void disablePruning() { + pruningEnabled = false; pruner.shutdownNow(); } + /** + * @return The number of threads that the pruner uses. This method is package-private because the tests must show + * that no thread runs when the pool has no buffers to prune. JAVA-6279 is about that behavior. + */ + int prunerThreadCount() { + return pruner.getPoolSize(); + } + @Override public ByteBuf getBuffer(final int size) { return new PooledByteBufNIO(getByteBuffer(size)); @@ -136,7 +180,59 @@ public void release(final ByteBuffer buffer) { powerOfTwoToPoolMap.get(log2(roundUpToNextHighestPowerOfTwo(buffer.capacity()))); if (pool != null) { pool.release(new IdleTrackingByteBuffer(buffer)); + startPruningIfNeeded(); + } + } + + private void startPruningIfNeeded() { + if (pruningEnabled && pruningScheduled.compareAndSet(false, true)) { + schedulePrune(); + } + } + + private void schedulePrune() { + try { + pruner.schedule(this::pruneAndRescheduleIfNeeded, maxIdleTimeNanos / 2, TimeUnit.NANOSECONDS); + } catch (RejectedExecutionException e) { + // Another thread called `disablePruning` and stopped the executor. A release of a buffer must not fail + // because of this. + pruningScheduled.set(false); + } + } + + /** + * Prunes the pool. Then schedules the next prune, but only if the pool is not empty. + * + *This method does not cancel a task to stop the pruner. It stops the pruner when it does not schedule the next + * prune. Then the work queue becomes empty and the pruner thread stops.
+ * + *The steps below prevent a lost pruner. A thread that releases a buffer reads {@link #pruningScheduled}. If + * that flag is true, the thread does not schedule a prune, because it relies on this method to schedule the next + * prune. For this reason, this method clears the flag and then examines the pool one more time. If the pool is not + * empty, this method takes the next prune. If it cannot take the next prune, the other thread has taken it. The + * class {@code io.netty.util.concurrent.GlobalEventExecutor.TaskRunner} uses the same steps.
+ */ + private void pruneAndRescheduleIfNeeded() { + prune(); + if (allPoolsEmpty()) { + pruningScheduled.set(false); + if (allPoolsEmpty()) { + return; + } + if (!pruningScheduled.compareAndSet(false, true)) { + return; + } + } + schedulePrune(); + } + + private boolean allPoolsEmpty() { + for (BufferPool pool : powerOfTwoToPoolMap.values()) { + if (!pool.isEmpty()) { + return false; + } } + return true; } private void prune() { @@ -204,5 +300,9 @@ void prune() { long now = System.nanoTime(); available.removeIf(cur -> now - cur.getLastUsedNanos() >= maxIdleTimeNanos); } + + boolean isEmpty() { + return available.isEmpty(); + } } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/PowerOfTwoBufferPoolTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/PowerOfTwoBufferPoolTest.java index e2b439ba6c..3cf50c8007 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/PowerOfTwoBufferPoolTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/PowerOfTwoBufferPoolTest.java @@ -22,10 +22,12 @@ import java.nio.ByteBuffer; import java.util.concurrent.TimeUnit; +import java.util.function.BooleanSupplier; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; public class PowerOfTwoBufferPoolTest { private PowerOfTwoBufferPool pool; @@ -75,7 +77,6 @@ public void testHugeBufferRequest() { assertNotSame(buf, pool.getBuffer((int) Math.pow(2, 10) + 1)); } - // Racy test @Test public void testPruning() throws InterruptedException { PowerOfTwoBufferPool pool = new PowerOfTwoBufferPool(10, 5, TimeUnit.MILLISECONDS) @@ -84,11 +85,88 @@ public void testPruning() throws InterruptedException { ByteBuf byteBuf = pool.getBuffer(256); ByteBuffer wrappedByteBuf = byteBuf.asNIO(); byteBuf.release(); - Thread.sleep(50); + // The pruner stops only after it empties the pool. Therefore a thread count of zero shows that the pruner + // removed the buffer. A wait for a fixed period would make this test racy. + assertTrue("the pruner must empty the pool", await(() -> pool.prunerThreadCount() == 0)); ByteBuf newByteBuf = pool.getBuffer(256); assertNotSame(wrappedByteBuf, newByteBuf.asNIO()); } finally { pool.disablePruning(); } } + + /** + * The pruner removes idle buffers, and an empty pool has no idle buffers. Therefore {@code enablePruning} must not + * start a thread. A thread that runs keeps the class loader of all driver classes in memory. See JAVA-6279. + */ + @Test + public void testEnablePruningStartsNoThreadWhileThePoolIsEmpty() { + PowerOfTwoBufferPool pool = new PowerOfTwoBufferPool(10, 5, TimeUnit.MILLISECONDS).enablePruning(); + try { + assertEquals(0, pool.prunerThreadCount()); + } finally { + pool.disablePruning(); + } + } + + /** + * The pruner empties the pool. Then it has no more work, and the thread must stop. The thread must not continue to + * wake up. This behavior is the correction for JAVA-6279. + */ + @Test + public void testPrunerThreadTerminatesOnceThePoolIsDrained() throws InterruptedException { + PowerOfTwoBufferPool pool = new PowerOfTwoBufferPool(10, 5, TimeUnit.MILLISECONDS).enablePruning(); + try { + pool.getBuffer(256).release(); + assertTrue("the pruner thread should terminate once the pool is drained", + await(() -> pool.prunerThreadCount() == 0)); + } finally { + pool.disablePruning(); + } + } + + /** + * The pruner must start again. A pool can become idle and then busy. If the pruner does not start again, the pool + * keeps the buffers that you release after the idle period. + */ + @Test + public void testPruningResumesAfterTheThreadHasTerminated() throws InterruptedException { + PowerOfTwoBufferPool pool = new PowerOfTwoBufferPool(10, 5, TimeUnit.MILLISECONDS).enablePruning(); + try { + pool.getBuffer(256).release(); + assertTrue("precondition: the pruner thread terminates once drained", + await(() -> pool.prunerThreadCount() == 0)); + + ByteBuf byteBuf = pool.getBuffer(256); + ByteBuffer wrapped = byteBuf.asNIO(); + byteBuf.release(); + assertTrue("a buffer released after termination should still be pruned", + await(() -> pool.getBuffer(256).asNIO() != wrapped)); + } finally { + pool.disablePruning(); + } + } + + /** A pool without pruning must not start a pruner thread. The number of buffers does not change this behavior. */ + @Test + public void testPruningDisabledPoolNeverStartsAThread() { + ByteBuf byteBuf = pool.getBuffer(256); + ByteBuffer wrapped = byteBuf.asNIO(); + byteBuf.release(); + // This assertion needs no wait. The executor creates its worker thread when it accepts a task, and not when it + // runs that task. Therefore a pool that schedules a prune has a thread before `release` returns. + assertEquals(0, pool.prunerThreadCount()); + assertSame("the pool must keep the buffer because it does not prune", wrapped, pool.getBuffer(256).asNIO()); + } + + private static boolean await(final BooleanSupplier condition) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (System.nanoTime() < deadline) { + if (condition.getAsBoolean()) { + return true; + } + Thread.sleep(5); + } + return condition.getAsBoolean(); + } } diff --git a/testing/java-6279-poc/FINDINGS.md b/testing/java-6279-poc/FINDINGS.md new file mode 100644 index 0000000000..b3e67a7bb9 --- /dev/null +++ b/testing/java-6279-poc/FINDINGS.md @@ -0,0 +1,668 @@ +# JAVA-6279 — class loader retention findings + +Internal working notes for [JAVA-6279](https://jira.mongodb.org/browse/JAVA-6279) (*Stop BufferPoolPruner thread when last MongoClient +closes*), [GitHub issue 2029](https://github.com/mongodb/mongo-java-driver/issues/2029) and +[JAVA-5643](https://jira.mongodb.org/browse/JAVA-5643). Touches JAVA-6240 (`CommonExecutor`) in §5. + +This directory began as a portable rework of Valentin Kovalenko's +[`primer` experiment](https://github.com/stIncMale/mongo-java-driver/commit/862b7d75fa0629e2b7c9cc4d6e8761b1678934dd), which hardcoded an +absolute path to one developer's `build/classes` directory and printed its results. It now resolves paths at run time, adds control +scenarios and a negative control, extends from synthetic classes to the driver itself, and compares every outcome against a stated +expectation so it can be run unattended. + +Claims are tagged **[executed]** = observed by running this code, **[code]** = read from source, **[unmeasured]** = not established either +way. + +Run it with `./testing/java-6279-poc/run.sh` (see [How to run](#how-to-run)). Nothing under any module's `src/main` is modified by the +harness. + +## 1. Verdict summary + +19 class loader scenarios and 12 executor mechanism checks, the latter under every JDK on the machine. Last full run: +all scenarios matched expectation, 50 PASS / 10 INFO / 0 FAIL. `PINNED` means the class loader was still strongly reachable after a window +of `System.gc()` nudges (10 s by default, `-Djava6279.gcWindowSeconds` to change it); +`COLLECTED` means its phantom reference was enqueued. + +### The two conclusions that matter + +1. **A non-terminated thread we start prevents the class loader of all driver classes — and therefore all of their static state — from being + collected.** **[executed]** +2. **`BufferPoolPruner` is that thread, and after `MongoClient.close()` it is the only thing left holding the loader.** + **[executed]** + +The second is load-bearing and is why `driver/OPEN_AND_CLOSE_CLIENT_THEN_DISABLE_PRUNING` exists. Knowing the pruner survives `close()` is +not enough — if anything *else* also survived, fixing the pruner would not release the loader. Nothing else does, so terminating it is +sufficient. + +### Driver scenarios + +| Scenario | Before the fix | After the fix | +|-----------------------------------------------------|----------------------------------------------------------|------------------------------------------------------------------------------| +| `driver/LOAD_ONLY` | COLLECTED | COLLECTED — control: loading driver classes leaks nothing | +| `driver/TOUCH_DEFAULT_POOL` | **PINNED** | **COLLECTED** — an empty pool now starts no thread | +| `driver/TOUCH_DEFAULT_POOL_THEN_DISABLE_PRUNING` | COLLECTED | COLLECTED | +| `driver/OPEN_AND_CLOSE_CLIENT` | **PINNED** | **COLLECTED after ~90 s** — the reported symptom, fixed | +| `driver/OPEN_AND_CLOSE_CLIENT_THEN_DISABLE_PRUNING` | COLLECTED | COLLECTED — before the fix, this was the proof the pruner was the *only* pin | +| `…_AND_TOUCH_COMMON_EXECUTOR` | skipped on `main`; **PINNED** on the backpressure branch | unchanged — `CommonExecutor` is a separate pin, see §5 | + +The ~90 s is inherent, not slack: with a one minute `maxIdleTime` a released buffer is only evictable after 60 s (two prune cycles at +`maxIdleTime / 2`), and the thread then times out after the keep-alive. **Any test of this fix must allow for that tail** — hence +`-Djava6279.driverGcWindowSeconds`, default 150. + +### Primer scenarios: what pins, and what does not + +| Scenario | Result | Isolates | +|-----------------------------------------------|------------|---------------------------------------------------------------------------------------------------------------------------------------| +| `primer/Inert` | COLLECTED | Control: the harness can observe a child loader being collected at all. | +| `primer/StartsOwnThread` | **PINNED** | A thread *constructed* in a child-loaded class's static initializer pins the loader, even with a parent-loaded `Runnable`. | +| `primer/StartsParentBuiltThread` | COLLECTED | Starting a `Thread` a *parent*-loaded class constructed does not pin. Isolates construction as the capture point. | +| `cclOnly/inherited` | **PINNED** | With **no child frame on the stack**, inheriting the child loader as context class loader pins it. A second, independent edge. | +| `cclOnly/nulled` | COLLECTED | Nulling that context class loader — *after* construction — closes that edge. | +| `primer/StartsOwnThreadNettyStyle` | **PINNED** | Netty's context class loader dance does not help when the thread's own class is in the loader. Confounded by the stack frame; see §2. | +| `primer/InheritsContextClassLoader` | **PINNED** | Confounded (stack frame present). Retained to show the stack capture dominates. | +| `primer/InheritsContextClassLoaderButNulled` | **PINNED** | Confounded, as above. | +| `primer/InheritsContextClassLoaderNettyDance` | **PINNED** | Confounded, as above — even nulling the *calling* thread's loader before construction cannot remove a stack frame. | +| `primer/RegistersShutdownHook` | **PINNED** | A shutdown hook pins even though the class starts no thread — see §7. | +| `primer/RegistersShutdownHookNettyStyle` | **PINNED** | Adding the context class loader nulling does not rescue it. | +| `primer/RegistersShutdownHookParentBody` | COLLECTED | The only non-pinning hook shape, and it cannot call driver code. | +| `primer/StaticSingletonExecutor` | **PINNED** | Models `CommonExecutor`, and shows the proposed `Cleaner` fix can never run — see §5. | + +### Executor mechanism checks + +Run at `--release 8` under **JDK 8, 11, 17, 23 (GraalVM) and 26**, because this leans on `ScheduledThreadPoolExecutor` +*implementation* behaviour rather than documented contract, and the driver's baseline is Java 8. + +| Check | Result | +|--------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------| +| The two settings alone do not reap the worker — a decision to stop is required | `poolSize=1, queue=1` while idle at 10× the keep-alive | +| One-shot scheduling self-terminates with no drain check (the `CommonExecutor` shape) | `poolSize=0` after the task runs, resurrects, back to 0 | +| A pending long one-shot delay survives a much shorter keep-alive | 3000 ms delay, 100 ms keep-alive → fired at 3002–3004 ms | +| Repeated resurrection is reliable over 500 create/reap cycles | 0 tasks lost, 500 threads created | +| Concurrent scheduling across resurrection loses nothing (8 × 250) | all ran, 0 rejections, **1 thread created** | +| **The conditional-reschedule design holds under contention** | 0 orphaned pools, 0 threads left — and **7/20 rounds orphaned with the re-check removed** | +| Reaps the worker once a periodic task is cancelled | `poolSize` 1 → 0 | +| Stays reusable and resurrects on re-scheduling | `isShutdown=false`, a *new* thread created | +| Self-cancellation from inside the task stops the repeat, 2000 round trips | 0 lost, 0 repeats not stopped | +| *(INFO)* The future needs safe publication | 0–13 per 2000 round trips throw a swallowed `NullPointerException` | +| *(INFO)* Cancelled task retained in the queue without `removeOnCancelPolicy` | `queue=1, poolSize=1` on most JDKs; JDK 8 reached 0 anyway | +| A generous keep-alive avoids thread churn, 25 cycles | 1 ms → 25 threads; 2 s → 1 thread | + +## 2. What pins the loader: two independent edges + +There are **two** distinct retaining edges. Conflating them wasted time here, and they have different fixes. + +### Edge A — capture at thread construction + +`primer/StartsOwnThread` (PINNED) versus `primer/StartsParentBuiltThread` (COLLECTED) isolates this to the moment +`new Thread(...)` runs: + +- Both run the same parent-loaded `Poc.SLEEPING_RUNNABLE`, so the executing thread holds no reference into the child loader by way of its + task. **[code]** +- In both, the thread's context class loader is the *application* loader, not the child loader — the harness prints it. So edge B is not + what is acting here. **[executed]** +- The only difference is which class was on the stack when the constructor ran. That alone flips the outcome. **[executed]** + +`primer/StartsOwnThread` deliberately uses `new Thread(null, runnable, name, 1, false)` — no thread group, no inherited thread locals — so +the pinning cannot be attributed to inherited state. **[code]** + +The exact retaining field inside `java.lang.Thread` was not identified. It was not needed, and it is a JDK implementation detail rather than +a contract. **[unmeasured]** + +**Consequence:** a thread constructed from driver code pins the driver's own loader, and driver code is on that stack by definition. Nothing +can be nulled or cleared away. **The thread has to actually terminate.** This is JAVA-6279. + +### Edge B — the inherited context class loader + +`cclOnly/*` isolates this by constructing the thread from `Poc`, with **no child-loaded frame on the stack**, while the calling thread's +context class loader is the child loader. So edge A is absent and only edge B can act: + +| Scenario | Result | +|---------------------------------------------------------------------|---------------| +| `cclOnly/inherited` | **PINNED** | +| `cclOnly/nulled` — context class loader nulled *after* construction | **COLLECTED** | + +So the context class loader is a genuine independent edge, and nulling it closes it. **Nulling after construction is sufficient**; Netty's +dance around the calling thread is not required for this edge. **[executed]** + +**Consequence:** this is the edge where a driver thread created on an application thread's behalf pins the *application's* loader. +`t.setContextClassLoader(null)` in `DaemonThreadFactory.newThread` closes it. That is a distinct bug from this ticket, and the change is +**not** in the tree — see the recommendation in §8. + +### Why the `primer/Inherits*` scenarios are retained but prove nothing about edge B + +Those three scenarios attempt edge B from inside a child-loaded class's `The alternative -- {@code shutdown()} when drained -- is terminal: a shut-down + * {@link ScheduledThreadPoolExecutor} rejects further submissions, so resurrection would mean building a new executor + * each cycle, which in turn means a non-final field and a lock guarding it. The recipe checked here avoids all of + * that:
+ * + *+ * ScheduledThreadPoolExecutor pruner = new ScheduledThreadPoolExecutor(1, factory); + * pruner.setKeepAliveTime(keepAlive, unit); // must be > 0 + * pruner.allowCoreThreadTimeOut(true); // let the core worker die when idle + * pruner.setRemoveOnCancelPolicy(true); // so a cancelled periodic task leaves the queue empty + *+ * + *
{@code prune()} then cancels its own periodic future when it finds the pool drained, the worker times out and + * exits, and a later {@code scheduleAtFixedRate} on the same executor brings a worker back.
+ * + *Deliberately Java 8 clean, with no dependency on the rest of this proof of concept, so that {@code run.sh} can + * compile it at {@code --release 8} and run it under every JDK on the machine. The driver's baseline is Java 8 and + * the behaviour being relied on is unspecified {@link ScheduledThreadPoolExecutor} implementation behaviour, not + * contract, so "it works on the developer's JDK" is not good enough.
+ */ +public final class ExecutorMechanism { + /** + * Big enough to be meaningful: the safe-publication hazard below shows up in roughly 0.5% of round trips, so a + * sample of 200 reports green about a third of the time. Sample sizes here are chosen against measured rates. + */ + private static final int ROUND_TRIPS = 2000; + + /** + * Set {@code -Djava6279.breakRecheck=true} to omit the check / re-check from + * {@link #conditionalRescheduleDesignHoldsUnderContention()}. That check must then FAIL; if it still passes, it is + * not sensitive enough to be evidence of anything. + */ + private static final boolean BREAK_RECHECK = Boolean.getBoolean("java6279.breakRecheck"); + + private ExecutorMechanism() { + } + + public static void main(final String... args) throws Exception { + System.out.printf("%s %s by %s%n", System.getProperty("java.vm.name"), System.getProperty("java.version"), + System.getProperty("java.vendor")); + ListThe {@code published} latch is not ceremony. {@code scheduleAtFixedRate} can start running the task + * before it returns the future, so a task that reads a field written *after* the call can see the unwritten + * value. See {@link #safePublicationOfTheFutureIsRequired()} — this cost an afternoon.
+ */ + ScheduledFuture> scheduleSelfCancelling(final CountDownLatch ran) { + final AtomicReference{@code enablePruning()} uses {@code scheduleAtFixedRate}, so the periodic task sits in the + * {@code DelayedWorkQueue} permanently. The worker therefore always has something to wait for, {@code getTask} + * never returns null, and the keep-alive never expires — the settings are inert. {@code removeOnCancelPolicy} is + * likewise inert, because nothing ever cancels anything.
+ * + *Note this is not specific to {@code scheduleAtFixedRate}: a self-rescheduling one-shot has the same property, + * since the next run is queued before the current one ends. The queue is only empty when pruning has genuinely + * stopped, which is the point — somebody has to decide to stop. That decision, the drained check, is the + * actual fix; these two settings are only what turns the decision into a dead thread.
+ */ + private static Check settingsAloneDoNotReapTheWorker() throws Exception { + Pruner pruner = new Pruner(100, TimeUnit.MILLISECONDS, true); + try { + final CountDownLatch ran = new CountDownLatch(1); + // A period long relative to the keep-alive, as the real one is: 1 minute idle time, 30 second period. + ScheduledFuture> task = pruner.executor.scheduleAtFixedRate(new Runnable() { + @Override + public void run() { + ran.countDown(); + } + }, 0, 2, TimeUnit.SECONDS); + if (!ran.await(5, TimeUnit.SECONDS)) { + return Check.fail("the two settings alone do not reap the worker", "the task never ran"); + } + // Idle for many multiples of the keep-alive, in the gap between two runs. + Thread.sleep(1000); + int poolSize = pruner.executor.getPoolSize(); + int queued = pruner.executor.getQueue().size(); + task.cancel(false); + int afterCancel = awaitPoolSize(pruner, 0); + // Passes by demonstrating that the settings are inert until something cancels. + return Check.of("the two settings alone do not reap the worker -- a decision to stop is required", + poolSize == 1 && queued == 1 && afterCancel == 0, + "idle 10x the keep-alive with the periodic task still scheduled: poolSize=" + poolSize + + ", queue=" + queued + " (thread alive, loader still pinned); " + + "poolSize=" + afterCancel + " only once the task stops being scheduled"); + } finally { + pruner.shutdownNow(); + } + } + + /** + * The distinction that decides how much work each fix is: one-shot scheduling needs no drain check at all. + * + *{@code PowerOfTwoBufferPool.enablePruning()} uses {@code scheduleAtFixedRate}, so its queue is never empty and + * {@code allowCoreThreadTimeOut} can never fire — see {@link #settingsAloneDoNotReapTheWorker()}. But + * {@code CommonExecutor.schedule} uses one-shot {@code schedule(...)}, so once the scheduled task has run the queue + * really is empty, the worker times out on its own, and nothing has to decide to stop. For that shape the two + * settings ARE the whole fix, with none of the stop-versus-release race.
+ */ + private static Check oneShotSchedulingSelfTerminatesWithNoDrainCheck() throws Exception { + Pruner pruner = new Pruner(100, TimeUnit.MILLISECONDS, true); + try { + final CountDownLatch ran = new CountDownLatch(1); + pruner.executor.schedule(new Runnable() { + @Override + public void run() { + ran.countDown(); + } + }, 10, TimeUnit.MILLISECONDS); + if (!ran.await(5, TimeUnit.SECONDS)) { + return Check.fail("one-shot scheduling self-terminates with no drain check", "the task never ran"); + } + int afterRun = awaitPoolSize(pruner, 0); + // And it must still resurrect for the next scheduled task. + final CountDownLatch ranAgain = new CountDownLatch(1); + pruner.executor.schedule(new Runnable() { + @Override + public void run() { + ranAgain.countDown(); + } + }, 10, TimeUnit.MILLISECONDS); + boolean resurrected = ranAgain.await(5, TimeUnit.SECONDS); + int afterSecond = awaitPoolSize(pruner, 0); + return Check.of("one-shot scheduling self-terminates with no drain check (the CommonExecutor shape)", + afterRun == 0 && resurrected && afterSecond == 0 && pruner.threadsCreated.get() > 1, + "poolSize=" + afterRun + " after the one-shot ran, task ran again=" + resurrected + + ", poolSize=" + afterSecond + " after that, threads ever created=" + + pruner.threadsCreated.get()); + } finally { + pruner.shutdownNow(); + } + } + + /** + * The safety question for the {@code CommonExecutor} fix: with {@code allowCoreThreadTimeOut(true)} and a keep-alive + * much SHORTER than a pending one-shot delay, is that pending task still honoured, or can the worker time out and + * drop it? + * + *This matters because {@code sleepAsync} delays are arbitrary — a retry backoff may be seconds while a sensible + * keep-alive is shorter. Losing a pending task would hang the callback, which is far worse than a leaked thread.
+ * + *Safe by construction, per {@code ThreadPoolExecutor.processWorkerExit}: if the last worker exits while the + * queue is non-empty, a replacement is added. Checked anyway.
+ */ + private static Check pendingLongDelaySurvivesAShortKeepAlive() throws Exception { + Pruner pruner = new Pruner(100, TimeUnit.MILLISECONDS, true); + try { + long delayMillis = 3000; + final CountDownLatch ran = new CountDownLatch(1); + long scheduledAt = System.nanoTime(); + pruner.executor.schedule(new Runnable() { + @Override + public void run() { + ran.countDown(); + } + }, delayMillis, TimeUnit.MILLISECONDS); + boolean honoured = ran.await(delayMillis * 3, TimeUnit.MILLISECONDS); + long actualMillis = (System.nanoTime() - scheduledAt) / 1_000_000L; + int afterRun = awaitPoolSize(pruner, 0); + // Late is as bad as lost for a callback, so require it within a generous window of the requested delay. + boolean onTime = honoured && actualMillis < delayMillis * 2; + return Check.of("a pending long one-shot delay survives a much shorter keep-alive", + onTime && afterRun == 0, + "keep-alive=100ms, delay=" + delayMillis + "ms, ran=" + honoured + " after " + actualMillis + + "ms, poolSize=" + afterRun + " once it had run"); + } finally { + pruner.shutdownNow(); + } + } + + /** + * {@code CommonExecutor} is a singleton shared by every {@code MongoClient}, so with a short keep-alive its worker + * may be created and reaped over and over. Two questions: is that reliable, and what does it cost? + * + *Each iteration schedules a one-shot, waits for it, then waits for the pool to drain to zero — so every + * iteration crosses the die/resurrect boundary deliberately, which is the worst case rather than the typical one.
+ */ + private static Check repeatedResurrectionIsReliable() throws Exception { + Pruner pruner = new Pruner(1, TimeUnit.MILLISECONDS, true); + try { + int iterations = 500; + int notRun = 0; + long startNanos = System.nanoTime(); + for (int i = 0; i < iterations; i++) { + final CountDownLatch ran = new CountDownLatch(1); + pruner.executor.schedule(new Runnable() { + @Override + public void run() { + ran.countDown(); + } + }, 0, TimeUnit.MILLISECONDS); + if (!ran.await(5, TimeUnit.SECONDS)) { + notRun++; + } + awaitPoolSize(pruner, 0); + } + long elapsedMicrosPerCycle = (System.nanoTime() - startNanos) / 1000L / iterations; + int threads = pruner.threadsCreated.get(); + return Check.of("repeated resurrection is reliable over " + iterations + " create/reap cycles", + notRun == 0 && threads > iterations / 2, + "tasks never run=" + notRun + ", threads ever created=" + threads + + " (churn really happened), ~" + elapsedMicrosPerCycle + "us per full cycle"); + } finally { + pruner.shutdownNow(); + } + } + + /** + * The multi-client case: several threads scheduling concurrently while the worker is dying. If resurrection lost a + * task here, a {@code sleepAsync} callback would never complete — a hang, not a leak. + */ + private static Check concurrentSchedulingAcrossResurrection() throws Exception { + final Pruner pruner = new Pruner(1, TimeUnit.MILLISECONDS, true); + try { + final int producers = 8; + final int perProducer = 250; + final CountDownLatch allRan = new CountDownLatch(producers * perProducer); + final CountDownLatch go = new CountDownLatch(1); + final AtomicInteger rejected = new AtomicInteger(); + Thread[] threads = new Thread[producers]; + for (int p = 0; p < producers; p++) { + threads[p] = new Thread(new Runnable() { + @Override + public void run() { + try { + go.await(); + for (int i = 0; i < perProducer; i++) { + try { + pruner.executor.schedule(new Runnable() { + @Override + public void run() { + allRan.countDown(); + } + }, 0, TimeUnit.MILLISECONDS); + } catch (java.util.concurrent.RejectedExecutionException e) { + rejected.incrementAndGet(); + allRan.countDown(); + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + }, "producer-" + p); + threads[p].start(); + } + go.countDown(); + boolean everythingRan = allRan.await(30, TimeUnit.SECONDS); + for (Thread t : threads) { + t.join(5000); + } + int afterwards = awaitPoolSize(pruner, 0); + return Check.of("concurrent scheduling across resurrection loses nothing (" + + producers + " threads x " + perProducer + ")", + everythingRan && rejected.get() == 0 && afterwards == 0, + "all tasks ran=" + everythingRan + ", rejections=" + rejected.get() + + ", outstanding=" + allRan.getCount() + ", poolSize afterwards=" + afterwards + + ", threads ever created=" + pruner.threadsCreated.get()); + } finally { + pruner.shutdownNow(); + } + } + + /** + * Prototypes the design this points to for {@code PowerOfTwoBufferPool}: replace {@code scheduleAtFixedRate} with a + * one-shot {@code schedule} that conditionally reschedules itself — next run only if the pool still holds + * something. Draining then makes the queue empty all by itself, so the worker times out; a later release schedules + * again. + * + *Two advantages over cancelling one's own periodic future:
+ *The stop-versus-release race remains and still needs check / re-check. This models it with Netty's + * {@code GlobalEventExecutor} protocol and asserts the invariant that actually matters: once everything is + * quiescent, the pool must be empty (nothing was orphaned) and no thread may remain.
+ */ + private static Check conditionalRescheduleDesignHoldsUnderContention() throws Exception { + final Pruner pruner = new Pruner(50, TimeUnit.MILLISECONDS, true); + try { + int rounds = 20; + int orphanedRounds = 0; + int threadLeftRounds = 0; + for (int round = 0; round < rounds; round++) { + final java.util.concurrent.ConcurrentLinkedDeque