From b9e04fd8c29c99ed848af0e75d481eb0fc89e93e Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Thu, 25 Jun 2026 07:45:17 -0600 Subject: [PATCH 01/14] Introduce `sleepAsync` JAVA-6240 --- config/spotbugs/exclude.xml | 27 +- .../mongodb/MongoInterruptedException.java | 2 +- .../connection/AsyncTransportSettings.java | 14 +- .../connection/NettyTransportSettings.java | 5 +- .../src/main/com/mongodb/internal/Locks.java | 5 +- .../internal/async/SingleResultCallback.java | 2 +- .../async/function/AsyncCallbackFunction.java | 2 +- ...nousSocketChannelStreamFactoryFactory.java | 43 +++- .../connection/DefaultConnectionPool.java | 3 +- .../connection/StreamFactoryFactory.java | 5 +- .../connection/StreamFactoryHelper.java | 6 + .../TlsChannelStreamFactoryFactory.java | 26 +- .../netty/NettyStreamFactoryFactory.java | 30 ++- .../internal/diagnostics/logging/Logger.java | 3 +- .../internal/thread/AsyncClientExecutor.java | 85 +++++++ .../internal/thread/CommonExecutor.java | 117 +++++++++ .../internal/thread/DaemonThreadFactory.java | 6 +- .../thread/DefaultAsyncClientExecutor.java | 190 ++++++++++++++ .../internal/thread/InterruptionUtil.java | 6 +- .../MongoScheduledThreadPoolExecutor.java | 50 ++++ .../thread/MongoThreadPoolExecutor.java | 94 +++++++ .../UnimplementedAsyncClientExecutor.java | 52 ++++ ...ternalStreamConnectionSpecification.groovy | 12 +- .../connection/SocksSocketFunctionalTest.java | 2 +- .../internal/thread/CommonExecutorTest.java | 58 +++++ .../DefaultAsyncClientExecutorTest.java | 237 ++++++++++++++++++ .../MongoScheduledThreadPoolExecutorTest.java | 58 +++++ .../thread/MongoThreadPoolExecutorTest.java | 125 +++++++++ .../UnimplementedAsyncClientExecutorTest.java | 59 +++++ .../client/internal/MongoClientImpl.java | 28 +-- 30 files changed, 1280 insertions(+), 72 deletions(-) create mode 100644 driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java create mode 100644 driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java create mode 100644 driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java create mode 100644 driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java create mode 100644 driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java create mode 100644 driver-core/src/main/com/mongodb/internal/thread/UnimplementedAsyncClientExecutor.java create mode 100644 driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java create mode 100644 driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java create mode 100644 driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java create mode 100644 driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java create mode 100644 driver-core/src/test/unit/com/mongodb/internal/thread/UnimplementedAsyncClientExecutorTest.java diff --git a/config/spotbugs/exclude.xml b/config/spotbugs/exclude.xml index 20684680865..e2fd0ae362c 100644 --- a/config/spotbugs/exclude.xml +++ b/config/spotbugs/exclude.xml @@ -15,9 +15,13 @@ --> + + + + + + + + + + + + diff --git a/driver-core/src/main/com/mongodb/MongoInterruptedException.java b/driver-core/src/main/com/mongodb/MongoInterruptedException.java index e0adce7978c..774fb430a8c 100644 --- a/driver-core/src/main/com/mongodb/MongoInterruptedException.java +++ b/driver-core/src/main/com/mongodb/MongoInterruptedException.java @@ -29,7 +29,7 @@ /** * A driver-specific non-checked counterpart to {@link InterruptedException}. - * Before this exception is thrown, the {@linkplain Thread#isInterrupted() interrupt status} of the thread will have been set + * Before this exception is thrown, the {@linkplain Thread#isInterrupted() interrupted status} of the thread will have been set * unless the {@linkplain #getCause() cause} is {@link InterruptedIOException}, in which case the driver leaves the status as is. *

* The Java SE API uses exceptions different from {@link InterruptedException} to communicate the same information:

diff --git a/driver-core/src/main/com/mongodb/connection/AsyncTransportSettings.java b/driver-core/src/main/com/mongodb/connection/AsyncTransportSettings.java index 8e259392313..489ada57020 100644 --- a/driver-core/src/main/com/mongodb/connection/AsyncTransportSettings.java +++ b/driver-core/src/main/com/mongodb/connection/AsyncTransportSettings.java @@ -52,13 +52,15 @@ private Builder() { } /** - * The executor service, intended to be used exclusively by the mongo - * client. Closing the mongo client will result in {@linkplain ExecutorService#shutdown() orderly shutdown} - * of the executor service. - * - *

When {@linkplain SslSettings#isEnabled() TLS is not enabled}, see + * The {@link ExecutorService}, intended to be used exclusively by the {@code MongoClient}. + *

+ * {@linkplain AutoCloseable#close() Closing} the {@code MongoClient} results in + * {@linkplain ExecutorService#shutdown() orderly shutdown} of the {@code executorService}. + * The application must not directly shut down the {@code executorService}. + *

+ * When {@linkplain SslSettings#isEnabled() TLS is not enabled}, see * {@link java.nio.channels.AsynchronousChannelGroup#withThreadPool(ExecutorService)} - * for additional requirements for the executor service. + * for additional requirements for the {@code executorService}. * * @param executorService the executor service * @return this diff --git a/driver-core/src/main/com/mongodb/connection/NettyTransportSettings.java b/driver-core/src/main/com/mongodb/connection/NettyTransportSettings.java index cb3a7c7c090..c80ed298540 100644 --- a/driver-core/src/main/com/mongodb/connection/NettyTransportSettings.java +++ b/driver-core/src/main/com/mongodb/connection/NettyTransportSettings.java @@ -85,8 +85,9 @@ public Builder socketChannelClass(final Class socketCha /** * Sets the event loop group. - * - *

The application is responsible for shutting down the provided {@code eventLoopGroup}

+ *

+ * The application is responsible for shutting down the provided {@code eventLoopGroup}. + * It must not be shut down before or concurrently with {@linkplain AutoCloseable#close() closing} the {@code MongoClient}. * * @param eventLoopGroup the event loop group that all channels created by this factory will be a part of * @return this diff --git a/driver-core/src/main/com/mongodb/internal/Locks.java b/driver-core/src/main/com/mongodb/internal/Locks.java index 042fc9fd69f..66160636e22 100644 --- a/driver-core/src/main/com/mongodb/internal/Locks.java +++ b/driver-core/src/main/com/mongodb/internal/Locks.java @@ -26,7 +26,7 @@ import static com.mongodb.internal.thread.InterruptionUtil.interruptAndCreateMongoInterruptedException; /** - *

This class is not part of the public API and may be removed or changed at any time

+ * This class is not part of the public API and may be removed or changed at any time. */ public final class Locks { public static void withLock(final Lock lock, final Runnable action) { @@ -41,8 +41,7 @@ public static void withInterruptibleLock(final StampedLock lock, final Runnable try { stamp = lock.writeLockInterruptibly(); } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new MongoInterruptedException("Interrupted waiting for lock", e); + throw interruptAndCreateMongoInterruptedException("Interrupted waiting for lock", e); } try { runnable.run(); diff --git a/driver-core/src/main/com/mongodb/internal/async/SingleResultCallback.java b/driver-core/src/main/com/mongodb/internal/async/SingleResultCallback.java index 11da1c97f75..cefab04f312 100644 --- a/driver-core/src/main/com/mongodb/internal/async/SingleResultCallback.java +++ b/driver-core/src/main/com/mongodb/internal/async/SingleResultCallback.java @@ -37,7 +37,7 @@ public interface SingleResultCallback { * @param result the result, which may be null. Always null if e is not null. * @param t the throwable, or null if the operation completed normally * @throws RuntimeException Never. - * @throws Error Never, on the best effort basis. + * @throws Error Never, on the best-effort basis. */ void onResult(@Nullable T result, @Nullable Throwable t); diff --git a/driver-core/src/main/com/mongodb/internal/async/function/AsyncCallbackFunction.java b/driver-core/src/main/com/mongodb/internal/async/function/AsyncCallbackFunction.java index a869af91a39..3efe1fa97bf 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/AsyncCallbackFunction.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/AsyncCallbackFunction.java @@ -51,7 +51,7 @@ public interface AsyncCallbackFunction { * @param callback A consumer of a result, {@link SingleResultCallback#onResult(Object, Throwable) completed} after * (in the happens-before order) the asynchronous function completes. * @throws RuntimeException Never. Exceptions must be relayed to the {@code callback}. - * @throws Error Never, on the best effort basis. Errors should be relayed to the {@code callback}. + * @throws Error Never, on the best-effort basis. Errors should be relayed to the {@code callback}. */ void apply(P a, SingleResultCallback callback); } diff --git a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java index 8c5a8f654c5..cc54a6db9ee 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java @@ -16,12 +16,21 @@ package com.mongodb.internal.connection; +import com.mongodb.connection.AsyncTransportSettings; import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; +import com.mongodb.internal.diagnostics.logging.Loggers; +import com.mongodb.internal.thread.AsyncClientExecutor; +import com.mongodb.internal.thread.DaemonThreadFactory; +import com.mongodb.internal.thread.MongoThreadPoolExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; import java.nio.channels.AsynchronousChannelGroup; +import java.time.Duration; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.LinkedBlockingQueue; /** * A {@code StreamFactoryFactory} implementation for AsynchronousSocketChannel-based streams. @@ -32,6 +41,8 @@ public final class AsynchronousSocketChannelStreamFactoryFactory implements Stre private final InetAddressResolver inetAddressResolver; @Nullable private final AsynchronousChannelGroup group; + private final MongoThreadPoolExecutor ownedExecutorBackingClientExecutor; + private final AsyncClientExecutor clientExecutor; public AsynchronousSocketChannelStreamFactoryFactory(final InetAddressResolver inetAddressResolver) { this(inetAddressResolver, null); @@ -42,6 +53,12 @@ public AsynchronousSocketChannelStreamFactoryFactory(final InetAddressResolver i @Nullable final AsynchronousChannelGroup group) { this.inetAddressResolver = inetAddressResolver; this.group = group; + int availableProcessors = Runtime.getRuntime().availableProcessors(); + ownedExecutorBackingClientExecutor = new MongoThreadPoolExecutor( + availableProcessors, availableProcessors, Duration.ofMinutes(5), + new LinkedBlockingQueue<>(), new DaemonThreadFactory("ClientExecutor"), Loggers.getLogger("client")); + ownedExecutorBackingClientExecutor.allowCoreThreadTimeOut(true); + clientExecutor = AsyncClientExecutor.backedBy(ownedExecutorBackingClientExecutor); } @Override @@ -50,10 +67,32 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin inetAddressResolver, socketSettings, sslSettings, group); } + /** + * VAKOTODO create ticket, leave a TODO + * To make things right, this should be + * the {@linkplain AsyncClientExecutor} {@linkplain AsyncClientExecutor#backedBy(Executor) backed by} same {@link ExecutorService} that + * the {@link AsynchronousChannelGroup} in {@link AsynchronousSocketChannelStreamFactory} uses + * (see {@link #create(SocketSettings, SslSettings)}). + * That {@link ExecutorService} may be provided by an application via {@link AsyncTransportSettings#getExecutorService()}. + * But currently it is a separate {@link MongoThreadPoolExecutor}. + */ + @Override + public AsyncClientExecutor getClientExecutor() { + return clientExecutor; + } + @Override public void close() { - if (group != null) { - group.shutdown(); + try { + clientExecutor.close(); + } finally { + try { + if (group != null) { + group.shutdown(); + } + } finally { + ownedExecutorBackingClientExecutor.shutdown(); + } } } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java b/driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java index 2339cf18b86..a0a8bbe6e2d 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java +++ b/driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java @@ -894,7 +894,7 @@ public boolean shouldPrune(final UsageTrackingInternalConnection usageTrackingCo /** * Package-access methods are thread-safe, - * and only they should be called outside of the {@link OpenConcurrencyLimiter}'s code. + * and only they should be called outside the {@link OpenConcurrencyLimiter}'s code. */ @ThreadSafe private final class OpenConcurrencyLimiter { @@ -1334,7 +1334,6 @@ private boolean initUnlessClosed() { * {@linkplain Task#failAsClosed() fail} asynchronously. */ @Override - @SuppressWarnings("try") public void close() { withLock(lock, () -> { if (state != State.CLOSED) { diff --git a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java index 6cbe620fd43..8f3408d7bc5 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java @@ -18,6 +18,7 @@ import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; +import com.mongodb.internal.thread.AsyncClientExecutor; /** * A factory of {@code StreamFactory} instances. @@ -29,10 +30,12 @@ public interface StreamFactoryFactory extends AutoCloseable { * * @param socketSettings the socket settings * @param sslSettings the SSL settings - * @return a stream factory that will apply the given settins + * @return a stream factory that will apply the given settings */ StreamFactory create(SocketSettings socketSettings, SslSettings sslSettings); + AsyncClientExecutor getClientExecutor(); + @Override void close(); } diff --git a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java index 7aeb65720b0..75b7554d4d9 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java +++ b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java @@ -24,6 +24,7 @@ import com.mongodb.connection.SslSettings; import com.mongodb.connection.TransportSettings; import com.mongodb.internal.connection.netty.NettyStreamFactoryFactory; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; @@ -47,6 +48,11 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin return new SocketStreamFactory(inetAddressResolver, socketSettings, sslSettings); } + @Override + public AsyncClientExecutor getClientExecutor() { + return AsyncClientExecutor.unimplemented(); + } + @Override public void close() { //NOP diff --git a/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java index b0fae1d044d..b3d633221f3 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java @@ -20,6 +20,7 @@ import com.mongodb.MongoSocketOpenException; import com.mongodb.ServerAddress; import com.mongodb.connection.AsyncCompletionHandler; +import com.mongodb.connection.AsyncTransportSettings; import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; import com.mongodb.internal.connection.tlschannel.BufferAllocator; @@ -29,6 +30,7 @@ import com.mongodb.internal.connection.tlschannel.async.AsynchronousTlsChannelGroup; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; @@ -47,6 +49,7 @@ import java.security.NoSuchAlgorithmException; import java.util.Iterator; import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -68,6 +71,7 @@ public class TlsChannelStreamFactoryFactory implements StreamFactoryFactory { private final SelectorMonitor selectorMonitor; private final AsynchronousTlsChannelGroup group; + private final AsyncClientExecutor clientExecutor; private final PowerOfTwoBufferPool bufferPool = PowerOfTwoBufferPool.DEFAULT; private final InetAddressResolver inetAddressResolver; @@ -78,6 +82,7 @@ public class TlsChannelStreamFactoryFactory implements StreamFactoryFactory { @Nullable final ExecutorService executorService) { this.inetAddressResolver = inetAddressResolver; this.group = new AsynchronousTlsChannelGroup(executorService); + clientExecutor = AsyncClientExecutor.backedBy(group); selectorMonitor = new SelectorMonitor(); selectorMonitor.start(); } @@ -93,10 +98,27 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin selectorMonitor); } + /** + * The {@linkplain AsyncClientExecutor} {@linkplain AsyncClientExecutor#backedBy(Executor) backed by} the same {@link ExecutorService} + * used by {@link #create(SocketSettings, SslSettings)} for a {@link StreamFactory}. + * That {@link ExecutorService} may be provided by an application via {@link AsyncTransportSettings#getExecutorService()}. + */ + @Override + public AsyncClientExecutor getClientExecutor() { + return clientExecutor; + } + @Override public void close() { - selectorMonitor.close(); - group.shutdown(); + try { + clientExecutor.close(); + } finally { + try { + selectorMonitor.close(); + } finally { + group.shutdown(); + } + } } /** diff --git a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java index 7fe54defaa2..6a31ef3a9a0 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java @@ -22,6 +22,7 @@ import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.connection.StreamFactory; import com.mongodb.internal.connection.StreamFactoryFactory; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; import io.netty.buffer.ByteBufAllocator; @@ -36,6 +37,7 @@ import java.security.Security; import java.util.Objects; +import java.util.concurrent.Executor; import static com.mongodb.assertions.Assertions.isTrueArgument; import static com.mongodb.assertions.Assertions.notNull; @@ -48,6 +50,7 @@ public final class NettyStreamFactoryFactory implements StreamFactoryFactory { private final EventLoopGroup eventLoopGroup; private final boolean ownsEventLoopGroup; + private final AsyncClientExecutor clientExecutor; private final Class socketChannelClass; private final ByteBufAllocator allocator; @Nullable @@ -151,7 +154,7 @@ public Builder eventLoopGroup(final EventLoopGroup eventLoopGroup) { /** * Sets a {@linkplain SslContextBuilder#forClient() client-side} {@link SslContext io.netty.handler.ssl.SslContext}, * which overrides the standard {@link SslSettings#getContext()}. - * By default it is {@code null} and {@link SslSettings#getContext()} is at play. + * By default, it is {@code null} and {@link SslSettings#getContext()} is at play. *

* This option may be used as a convenient way to utilize * OpenSSL as an alternative to the TLS/SSL protocol implementation in a JDK. @@ -203,13 +206,27 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin sslContext); } + /** + * The {@linkplain AsyncClientExecutor} {@linkplain AsyncClientExecutor#backedBy(Executor) backed by} the same {@link EventLoopGroup} + * used by {@link #create(SocketSettings, SslSettings)} for a {@link StreamFactory}. + * That {@link EventLoopGroup} may be provided by an application via {@link NettyTransportSettings#getEventLoopGroup()}. + */ + @Override + public AsyncClientExecutor getClientExecutor() { + return clientExecutor; + } + @Override public void close() { - if (ownsEventLoopGroup) { - // ignore the returned Future. This is in line with MongoClient behavior to not block waiting for connections to be returned - // to the pool - eventLoopGroup.shutdownGracefully(); - } + try { + clientExecutor.close(); + } finally { + if (ownsEventLoopGroup) { + // ignore the returned Future. This is in line with MongoClient behavior to not block waiting for connections to be returned + // to the pool + eventLoopGroup.shutdownGracefully(); + } + } } @Override @@ -236,6 +253,7 @@ private NettyStreamFactoryFactory(final Builder builder) { socketChannelClass = builder.socketChannelClass == null ? NioSocketChannel.class : builder.socketChannelClass; eventLoopGroup = builder.eventLoopGroup == null ? new NioEventLoopGroup() : builder.eventLoopGroup; ownsEventLoopGroup = builder.eventLoopGroup == null; + clientExecutor = AsyncClientExecutor.backedBy(eventLoopGroup); sslContext = builder.sslContext; inetAddressResolver = builder.inetAddressResolver; } diff --git a/driver-core/src/main/com/mongodb/internal/diagnostics/logging/Logger.java b/driver-core/src/main/com/mongodb/internal/diagnostics/logging/Logger.java index e9907bb3953..4c1eda53e11 100644 --- a/driver-core/src/main/com/mongodb/internal/diagnostics/logging/Logger.java +++ b/driver-core/src/main/com/mongodb/internal/diagnostics/logging/Logger.java @@ -17,8 +17,7 @@ package com.mongodb.internal.diagnostics.logging; /** - * This class is not part of the public API. It may be removed or changed at any time. - * + * This class is not part of the public API and may be removed or changed at any time. */ public interface Logger { /** diff --git a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java new file mode 100644 index 00000000000..29175cfefaf --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java @@ -0,0 +1,85 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.annotations.ThreadSafe; +import com.mongodb.assertions.Assertions; +import com.mongodb.internal.async.SingleResultCallback; +import com.mongodb.internal.connection.StreamFactoryFactory; + +import java.time.Duration; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; + +/** + * An executor for a {@code MongoClient} (currently, only for an asynchronous one). + * An implementation may use resources shared by multiple clients, if appropriate. + *

+ * May be used to execute internal code that is not blocking, or application code. + * When an asynchronous client is used, the application code we may execute is supposed to not be blocking, but we cannot enforce that. + * If an application violates the contract, it bears the responsibility. + *

+ * Purposefully not {@link ExecutorService}, because it does not manage the underlying resources, if any. + * They must be managed externally to {@link AsyncClientExecutor}. + * Nonetheless, it is still {@link AutoCloseable}. See {@link #close()} for the details. + *

+ * This class is not part of the public API and may be removed or changed at any time. + * + * @see StreamFactoryFactory#getClientExecutor() + * @see CommonExecutor + */ +@ThreadSafe +public interface AsyncClientExecutor extends AutoCloseable { + /** + * The returned {@link AsyncClientExecutor} must not be used, as its methods {@linkplain Assertions#fail() fail}. + */ + static AsyncClientExecutor unimplemented() { + return UnimplementedAsyncClientExecutor.instance(); + } + + /** + * @param executor The executor to use for executing tasks. + * If it is a {@link ScheduledExecutorService}, then it is also used for scheduling, + * otherwise {@link CommonExecutor} is used for scheduling. + */ + static AsyncClientExecutor backedBy(final Executor executor) { + return new DefaultAsyncClientExecutor(executor); + } + + /** + * The callback-based counterpart to {@link Thread#sleep(long, int)}. + * + * @param duration A non-{@linkplain Duration#isNegative() negative} duration. + * If {@code duration} is {@linkplain Duration#isZero() zero}, + * the {@code callback} is {@linkplain SingleResultCallback#complete(SingleResultCallback) completed} + * by the {@link Thread} that invoked the method. + * Regardless of the {@link Duration}, {@link #close()} causes the {@code callback} to be completed with + * {@link RejectedExecutionException}. + */ + void sleepAsync(Duration duration, SingleResultCallback callback); + + /** + * Must be called before shutting down the {@linkplain #backedBy(Executor) backing executor}, + * to notify this {@link AsyncClientExecutor} that the backing executor may be about to shut down. + * This method guarantees exactly-once {@linkplain SingleResultCallback#onResult(Object, Throwable) completion} + * of all callbacks that may have not been completed otherwise if the backing executor shuts down. + * An example of such a callback is one scheduled via {@link #sleepAsync(Duration, SingleResultCallback)}. + */ + @Override + void close(); +} diff --git a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java new file mode 100644 index 00000000000..9461023b01b --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java @@ -0,0 +1,117 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.connection.AsyncTransportSettings; +import com.mongodb.internal.diagnostics.logging.Logger; +import com.mongodb.internal.diagnostics.logging.Loggers; + +import java.time.Duration; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +import static com.mongodb.assertions.Assertions.fail; +import static java.util.concurrent.TimeUnit.NANOSECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; + +/** + * A per-{@link ClassLoader} executor. + *

+ * We do not always have access to {@link ScheduledExecutorService} in a {@code MongoClient}. + * For example, even if {@link AsyncTransportSettings#getExecutorService()} is present, it is merely an {@link ExecutorService}. + * {@link CommonExecutor} is always accessible and may be used to schedule tasks when a more suitable alternative does not exist, + * but must not be used to execute such scheduled tasks. + *

+ * Must not be used to execute blocking or application code. The only exception is {@link #uncaughtError(Error)}. + *

+ * This class is not part of the public API and may be removed or changed at any time. + */ +// VAKOTODO create a ticket and leave a TODO to use Cleaner when we are at Java SE 17 to shut down internal executors if the class is GCed. +public final class CommonExecutor { + private static final Logger LOGGER = Loggers.getLogger("client"); + private static final CommonExecutor INSTANCE = new CommonExecutor(); + + /** + * Exists solely to execute {@link ThreadGroup#uncaughtException(Thread, Throwable)}, + * which may execute application code and may be blocking. + * + * @see #uncaughtError(Error) + */ + private final ExecutorService uncaughtExceptionHandlerExecutor; + private final MongoScheduledThreadPoolExecutor scheduler; + + public static CommonExecutor commonExecutor() { + return INSTANCE; + } + + private CommonExecutor() { + // `uncaughtExceptionHandlerExecutor` is the only executor that must not be `MongoThreadPoolExecutor`/`MongoScheduledThreadPoolExecutor` + uncaughtExceptionHandlerExecutor = new ThreadPoolExecutor( + 0, 1, 5, SECONDS, new LinkedBlockingQueue<>(), new DaemonThreadFactory("CommonUncaughtExceptionHandler")); + scheduler = new MongoScheduledThreadPoolExecutor(1, new DaemonThreadFactory("CommonScheduler"), LOGGER); + } + + /** + * This method may be used in a situation when we cannot simply propagate an {@link Error} to the application. + * Handling an {@link Error} this way enables applications to decide how to deal with it via the + * {@linkplain Thread#getDefaultUncaughtExceptionHandler() default uncaught exception handler}. + *

+ * An {@link Error} is more likely to cause an invariant violation than an {@link Exception}, + * because it is less likely to be taken into account in code. An {@link AssertionError} outright informs about an invariant violation. + * Furthermore, a {@link VirtualMachineError} not only may happen in peculiar situations, + * but also may be asynchronous. + * That is why it may be a good idea for an application to terminate on {@link Error}. + * We cannot make such a decision for an application, but we must do our best to give it an opportunity to react to an {@link Error}. + *

+ * This method does not block. It also does not complete abruptly on the best-effort basis. + */ + void uncaughtError(final Error e) { + Thread t = Thread.currentThread(); + uncaughtExceptionHandlerExecutor.execute(() -> { + try { + t.getUncaughtExceptionHandler().uncaughtException(t, e); + } catch (Throwable ignored) { + // we are free to ignore the exception, and should ignore it + } + }); + } + + /** + * @param command The command to be scheduled. If it is {@link Executor#execute(Runnable) executed}, then the execution is guaranteed + * to be done via the {@code executor}. However, if the {@code executor} is shut down, then it does not execute the {@code command}, + * and there is nothing we can do about that, though {@link MongoScheduledThreadPoolExecutor#afterExecute(Runnable, Throwable)} + * logs the problem if {@link Executor#execute(Runnable)} completes abruptly. + * @param delay A non-{@linkplain Duration#isNegative() negative} delay. + * @param executor The {@link Executor} to use for {@code command} {@linkplain Runnable#run() execution}, + * so that the {@code command} is not executed by a thread managed by {@link CommonExecutor}. + * @return The {@link ScheduledFuture} representing only + * the {@linkplain ScheduledExecutorService#schedule(Runnable, long, TimeUnit) scheduling part}, + * and not the execution part done by the {@code executor}. + */ + ScheduledFuture schedule(final Runnable command, final Duration delay, final Executor executor) { + try { + return scheduler.schedule(() -> executor.execute(command), delay.toNanos(), NANOSECONDS); + } catch (RejectedExecutionException e) { + throw fail(e.toString()); + } + } +} diff --git a/driver-core/src/main/com/mongodb/internal/thread/DaemonThreadFactory.java b/driver-core/src/main/com/mongodb/internal/thread/DaemonThreadFactory.java index e44fd0661c4..e64890148af 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/DaemonThreadFactory.java +++ b/driver-core/src/main/com/mongodb/internal/thread/DaemonThreadFactory.java @@ -22,10 +22,10 @@ /** * Custom thread factory for scheduled executor service that creates daemon threads. Otherwise, * applications that neglect to close the client will not exit. - * - *

This class is not part of the public API and may be removed or changed at any time

+ *

+ * This class is not part of the public API and may be removed or changed at any time. */ -public class DaemonThreadFactory implements ThreadFactory { +public final class DaemonThreadFactory implements ThreadFactory { private static final AtomicInteger POOL_NUMBER = new AtomicInteger(1); private final AtomicInteger threadNumber = new AtomicInteger(1); private final String namePrefix; diff --git a/driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java new file mode 100644 index 00000000000..bf70de0bed0 --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java @@ -0,0 +1,190 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.annotations.NotThreadSafe; +import com.mongodb.annotations.ThreadSafe; +import com.mongodb.internal.async.SingleResultCallback; +import com.mongodb.lang.Nullable; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.locks.ReentrantLock; + +import static com.mongodb.assertions.Assertions.assertFalse; +import static com.mongodb.assertions.Assertions.assertNull; +import static com.mongodb.internal.Locks.withLock; +import static com.mongodb.internal.async.AsyncRunnable.beginAsync; +import static com.mongodb.internal.thread.CommonExecutor.commonExecutor; +import static java.lang.String.format; +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +@ThreadSafe +final class DefaultAsyncClientExecutor implements AsyncClientExecutor { + private final Executor backingExecutor; + private final Set scheduledCallbackCompletions; + /** + * While holding this lock, no application code may be executed, and driver code external to {@link DefaultAsyncClientExecutor} + * should be either avoided or carefully vetted. This is to avoid unexpected delays and deadlocks. + * For example, {@link ScheduledCallbackCompletion#reject(RejectedExecutionException)}, {@link ScheduledCallbackCompletion#run()} + * must not be executed while holding the lock. + */ + private final ReentrantLock closeLock; + private volatile boolean closed; + + DefaultAsyncClientExecutor(final Executor backingExecutor) { + this.backingExecutor = backingExecutor; + scheduledCallbackCompletions = ConcurrentHashMap.newKeySet(); + closeLock = new ReentrantLock(); + closed = false; + } + + @Override + public void sleepAsync(final Duration duration, final SingleResultCallback callback) { + sleepAsync(closed, duration, callback, () -> scheduleCompletion(callback, duration)); + } + + /** + * @param closed See {@link #closed}. + * @param onPositiveDurationDelayAndCompleteCallback Runs only if + * {@code duration} is {@linkplain Duration#isNegative() positive}. + * The {@link Runnable#run()} is allowed to complete abruptly and is guaranteed to run in the same thread that invoked this method. + */ + static void sleepAsync( + final boolean closed, + final Duration duration, + final SingleResultCallback callback, + final Runnable onPositiveDurationDelayAndCompleteCallback) { + beginAsync().thenRun(c -> { + assertFalse(duration.isNegative()); + if (closed) { + throw createClosedException(); + } + if (duration.isZero()) { + c.complete(c); + } else { + onPositiveDurationDelayAndCompleteCallback.run(); + } + }).finish(callback); + } + + private void scheduleCompletion(final SingleResultCallback callback, final Duration delay) { + ScheduledCallbackCompletion scheduledCallbackCompletion = new ScheduledCallbackCompletion(callback); + RejectedExecutionException rejectionCause = withLock(closeLock, () -> { + scheduledCallbackCompletions.add(scheduledCallbackCompletion); + if (closed) { + return createClosedException(); + } + ScheduledFuture scheduledFuture; + // We handle `isShutdown`, `RejectedExecutionException` below merely + // as the best effort to improve the application experience. + // Either situation violates the contract of the `close` method. + if (backingExecutor instanceof ExecutorService && ((ExecutorService) backingExecutor).isShutdown()) { + return createBackingExecutorShutdownException(); + } + if (backingExecutor instanceof ScheduledExecutorService) { + try { + scheduledFuture = ((ScheduledExecutorService) backingExecutor).schedule(scheduledCallbackCompletion, delay.toNanos(), NANOSECONDS); + } catch (RejectedExecutionException rejected) { + return rejected; + } + } else { + scheduledFuture = commonExecutor().schedule(scheduledCallbackCompletion, delay, backingExecutor); + } + scheduledCallbackCompletion.onScheduled(scheduledFuture); + return null; + }); + if (rejectionCause != null) { + scheduledCallbackCompletion.reject(rejectionCause); + } + } + + @Override + public void close() { + Collection localScheduledCallbackCompletions = new ArrayList<>(); + withLock(closeLock, () -> { + if (closed) { + return; + } + closed = true; + // Here we do not care about any `ScheduledCallbackCompletion` added after the current critical section, + // because its `reject` is called by the method that added it. + localScheduledCallbackCompletions.addAll(scheduledCallbackCompletions); + }); + for (ScheduledCallbackCompletion scheduledCallbackCompletion : localScheduledCallbackCompletions) { + scheduledCallbackCompletion.reject(createClosedException()); + } + } + + private static RejectedExecutionException createClosedException() { + return new RejectedExecutionException("Closed"); + } + + private RejectedExecutionException createBackingExecutorShutdownException() { + return new RejectedExecutionException(format("The backing executor %s is shut down", backingExecutor)); + } + + @Override + public String toString() { + return "DefaultAsyncClientExecutor{" + + "backingExecutor=" + backingExecutor + + ", closed=" + closed + + '}'; + } + + @NotThreadSafe + private class ScheduledCallbackCompletion implements Runnable { + private final SingleResultCallback callback; + @Nullable + private ScheduledFuture scheduledFuture; + + ScheduledCallbackCompletion(final SingleResultCallback callback) { + this.callback = callback; + } + + void onScheduled(final ScheduledFuture scheduledFuture) { + assertNull(this.scheduledFuture); + this.scheduledFuture = scheduledFuture; + } + + void reject(final RejectedExecutionException cause) { + if (scheduledCallbackCompletions.remove(this)) { + try { + if (scheduledFuture != null) { + scheduledFuture.cancel(false); + } + } finally { + callback.completeExceptionally(cause); + } + } + } + + @Override + public void run() { + if (scheduledCallbackCompletions.remove(this)) { + callback.complete(callback); + } + } + } +} diff --git a/driver-core/src/main/com/mongodb/internal/thread/InterruptionUtil.java b/driver-core/src/main/com/mongodb/internal/thread/InterruptionUtil.java index 54a3ba31f24..c933e951dee 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/InterruptionUtil.java +++ b/driver-core/src/main/com/mongodb/internal/thread/InterruptionUtil.java @@ -25,14 +25,14 @@ import java.util.Optional; /** - *

This class is not part of the public API and may be removed or changed at any time

+ * This class is not part of the public API and may be removed or changed at any time. */ public final class InterruptionUtil { /** * {@linkplain Thread#interrupt() Interrupts} the {@linkplain Thread#currentThread() current thread} * before creating {@linkplain MongoInterruptedException}. - * We do this because the interrupt status is cleared before throwing {@link InterruptedException}, - * we are not propagating {@link InterruptedException}, which means we must reinstate the interrupt status. + * We do this because the interrupted status is cleared before throwing {@link InterruptedException}, + * we are not propagating {@link InterruptedException}, which means we must reinstate the interrupted status. * This matches the behavior documented by {@link MongoInterruptedException}. */ public static MongoInterruptedException interruptAndCreateMongoInterruptedException( diff --git a/driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java new file mode 100644 index 00000000000..1ac2011f4ed --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java @@ -0,0 +1,50 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.internal.diagnostics.logging.Logger; +import com.mongodb.lang.Nullable; + +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ThreadFactory; + +import static com.mongodb.assertions.Assertions.assertNull; +import static com.mongodb.assertions.Assertions.assertTrue; +import static com.mongodb.internal.thread.MongoThreadPoolExecutor.handleTaskExceptions; + +/** + * See {@link MongoThreadPoolExecutor}. + */ +final class MongoScheduledThreadPoolExecutor extends ScheduledThreadPoolExecutor { + private final Logger logger; + + MongoScheduledThreadPoolExecutor( + final int corePoolSize, + final ThreadFactory threadFactory, + final Logger logger) { + super(corePoolSize, threadFactory); + setRemoveOnCancelPolicy(true); + this.logger = logger; + } + + @Override + protected void afterExecute(final Runnable r, @Nullable final Throwable t) { + super.afterExecute(r, t); + assertTrue(r instanceof Future); + handleTaskExceptions(r, assertNull(t), logger); + } +} diff --git a/driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java new file mode 100644 index 00000000000..05a46d00bbf --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java @@ -0,0 +1,94 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.internal.diagnostics.logging.Logger; +import com.mongodb.lang.Nullable; + +import java.time.Duration; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; + +import static com.mongodb.assertions.Assertions.assertNotNull; +import static com.mongodb.internal.thread.CommonExecutor.commonExecutor; +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +/** + * A {@link ThreadPoolExecutor} that always handles task exceptions, + * even if the caller ignores the {@link Future} that represents completion of the task: + *
    + *
  • + * {@link Error}s packed into {@link Future}s are handled via {@link CommonExecutor#uncaughtError(Error)}.
    • + *
  • + * All {@link Throwable}s are logged.
    • + *
+ */ +public final class MongoThreadPoolExecutor extends ThreadPoolExecutor { + private final Logger logger; + + public MongoThreadPoolExecutor( + final int corePoolSize, + final int maximumPoolSize, + final Duration keepAliveTime, + final BlockingQueue workQueue, + final ThreadFactory threadFactory, + final Logger logger) { + super(corePoolSize, maximumPoolSize, keepAliveTime.toNanos(), NANOSECONDS, workQueue, threadFactory); + this.logger = logger; + } + + @Override + protected void afterExecute(final Runnable r, @Nullable final Throwable t) { + super.afterExecute(r, t); + handleTaskExceptions(r, t, logger); + } + + static void handleTaskExceptions(final Runnable r, @Nullable final Throwable t, final Logger logger) { + Throwable exception = getException(r, t); + if (exception instanceof Error && t == null) { + // the `Error` is held in `r`, and would have not been thrown to be handled by the uncaught exception handler + commonExecutor().uncaughtError((Error) exception); + } + if (exception != null) { + logger.error("A task completed abruptly", exception); + } + } + + @Nullable + private static Throwable getException(final Runnable r, @Nullable final Throwable t) { + if (t != null) { + return t; + } + if (r instanceof Future) { + Future runnableFuture = (Future) r; + try { + runnableFuture.get(); + } catch (CancellationException e) { + return e; + } catch (ExecutionException e) { + return assertNotNull(e.getCause()); + } catch (InterruptedException e) { + // not else to do but to reinstate the interrupted status + Thread.currentThread().interrupt(); + } + } + return null; + } +} diff --git a/driver-core/src/main/com/mongodb/internal/thread/UnimplementedAsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/UnimplementedAsyncClientExecutor.java new file mode 100644 index 00000000000..8213fe47d73 --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/thread/UnimplementedAsyncClientExecutor.java @@ -0,0 +1,52 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.annotations.ThreadSafe; +import com.mongodb.internal.async.SingleResultCallback; + +import java.time.Duration; + +import static com.mongodb.assertions.Assertions.fail; + +@ThreadSafe +final class UnimplementedAsyncClientExecutor implements AsyncClientExecutor { + private static final UnimplementedAsyncClientExecutor INSTANCE = new UnimplementedAsyncClientExecutor(); + + static UnimplementedAsyncClientExecutor instance() { + return INSTANCE; + } + + private UnimplementedAsyncClientExecutor() { + } + + /** + * Must not be called with any {@link Duration} except {@linkplain Duration#isZero() zero}. + */ + @Override + public void sleepAsync(final Duration duration, final SingleResultCallback callback) { + DefaultAsyncClientExecutor.sleepAsync(false, duration, callback, () -> { + throw fail(); + }); + } + + /** + * Does nothing. + */ + @Override + public void close() { + } +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionSpecification.groovy b/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionSpecification.groovy index 10074ba9d3c..cb2c1cf8ac4 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionSpecification.groovy @@ -314,7 +314,7 @@ class InternalStreamConnectionSpecification extends Specification { connection.isClosed() } - def 'should throw MongoInterruptedException and leave the interrupt status set when Stream.write throws InterruptedIOException'() { + def 'should throw MongoInterruptedException and leave interrupted status set when Stream.write throws InterruptedIOException'() { given: stream.write(_, _) >> { throw new InterruptedIOException() } def connection = getOpenedConnection() @@ -329,7 +329,7 @@ class InternalStreamConnectionSpecification extends Specification { connection.isClosed() } - def 'should throw MongoInterruptedException and leave the interrupt status unset when Stream.write throws InterruptedIOException'() { + def 'should throw MongoInterruptedException and leave interrupted status unset when Stream.write throws InterruptedIOException'() { given: stream.write(_, _) >> { throw new InterruptedIOException() } def connection = getOpenedConnection() @@ -343,7 +343,7 @@ class InternalStreamConnectionSpecification extends Specification { connection.isClosed() } - def 'should throw MongoInterruptedException and leave the interrupt status set when Stream.write throws ClosedByInterruptException'() { + def 'should throw MongoInterruptedException and leave interrupted status set when Stream.write throws ClosedByInterruptException'() { given: stream.write(_, _) >> { throw new ClosedByInterruptException() } def connection = getOpenedConnection() @@ -386,7 +386,7 @@ class InternalStreamConnectionSpecification extends Specification { connection.isClosed() } - def 'should throw MongoInterruptedException and leave the interrupt status set when Stream.read throws InterruptedIOException'() { + def 'should throw MongoInterruptedException and leave interrupted status set when Stream.read throws InterruptedIOException'() { given: stream.read(_, _) >> { throw new InterruptedIOException() } def connection = getOpenedConnection() @@ -401,7 +401,7 @@ class InternalStreamConnectionSpecification extends Specification { connection.isClosed() } - def 'should throw MongoInterruptedException and leave the interrupt status unset when Stream.read throws InterruptedIOException'() { + def 'should throw MongoInterruptedException and leave interrupted status unset when Stream.read throws InterruptedIOException'() { given: stream.read(_, _) >> { throw new InterruptedIOException() } def connection = getOpenedConnection() @@ -415,7 +415,7 @@ class InternalStreamConnectionSpecification extends Specification { connection.isClosed() } - def 'should throw MongoInterruptedException and leave the interrupt status set when Stream.read throws ClosedByInterruptException'() { + def 'should throw MongoInterruptedException and leave interrupted status set when Stream.read throws ClosedByInterruptException'() { given: stream.read(_, _) >> { throw new ClosedByInterruptException() } def connection = getOpenedConnection() diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/SocksSocketFunctionalTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/SocksSocketFunctionalTest.java index 7bfba300a61..6d34f771e2f 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/SocksSocketFunctionalTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/SocksSocketFunctionalTest.java @@ -87,7 +87,7 @@ private void connectWithMiniServer(final byte[] serverBytes, final boolean withC t.join(CONNECT_TIMEOUT_MS); } catch (InterruptedException ie) { // Don't mask the primary exception (if any) with the join interruption; - // just preserve the thread's interrupt status and continue. + // just preserve the thread's interrupted status and continue. Thread.currentThread().interrupt(); } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java new file mode 100644 index 00000000000..5919543931a --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.lang.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.lang.Thread.UncaughtExceptionHandler; +import java.util.concurrent.CompletableFuture; + +import static com.mongodb.internal.thread.CommonExecutor.commonExecutor; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertNotSame; + +class CommonExecutorTest { + private static final long TIMEOUT_MILLIS = SECONDS.toMillis(1); + + @Nullable + private UncaughtExceptionHandler originalUncaughtExceptionHandler; + + @BeforeEach + void beforeEach() { + originalUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler(); + } + + @AfterEach + void afterEach() { + if (originalUncaughtExceptionHandler != null) { + Thread.setDefaultUncaughtExceptionHandler(originalUncaughtExceptionHandler); + } + } + + @Test + void uncaughtExceptionHandlerExecutesInDifferentThread() throws Exception { + CompletableFuture threadFuture = new CompletableFuture<>(); + Thread.setDefaultUncaughtExceptionHandler((t, e) -> { + threadFuture.complete(Thread.currentThread()); + }); + commonExecutor().uncaughtError(new AssertionError()); + assertNotSame(Thread.currentThread(), threadFuture.get(TIMEOUT_MILLIS, MILLISECONDS)); + } +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java new file mode 100644 index 00000000000..2771fbab2ad --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java @@ -0,0 +1,237 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.internal.time.StartTime; +import io.netty.channel.EventLoopGroup; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.time.Duration; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; + +import static com.mongodb.internal.thread.InterruptionUtil.interruptAndCreateMongoInterruptedException; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.NANOSECONDS; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class DefaultAsyncClientExecutorTest { + private static final long SLEEP_DURATION_MILLIS = 200; + + private ExecutorService executorService; + private ScheduledExecutorService scheduledExecutorService; + + @BeforeEach + void beforeEach() { + executorService = Executors.newSingleThreadExecutor(); + scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(); + } + + @AfterEach + void afterEach() { + if (executorService != null) { + executorService.shutdownNow(); + } + if (scheduledExecutorService != null) { + scheduledExecutorService.shutdownNow(); + } + } + + @ParameterizedTest + @ValueSource(longs = {0, SLEEP_DURATION_MILLIS}) + void sleepAsync(final long durationMs) { + Duration duration = Duration.ofMillis(durationMs); + try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); + DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { + assertAll( + () -> assertSleepAsync(backedByExecutorService, duration), + () -> assertSleepAsync(backedByScheduledExecutorService, duration) + ); + } + } + + private static void assertSleepAsync( + final DefaultAsyncClientExecutor clientExecutor, final Duration duration) throws Exception { + StartTime startTime = StartTime.now(); + CompletableFuture callbackDelayFuture = new CompletableFuture<>(); + CompletableFuture callbackThreadFuture = new CompletableFuture<>(); + clientExecutor.sleepAsync(duration, (result, t) -> { + if (t != null) { + callbackDelayFuture.completeExceptionally(t); + } else { + callbackDelayFuture.complete(startTime.elapsed()); + } + callbackThreadFuture.complete(Thread.currentThread()); + }); + // if `duration` is zero, the futures are expected to be completed synchronously + long timeoutMs = duration.toMillis() * 2; + Duration actualCallbackDelay = callbackDelayFuture.get(timeoutMs, MILLISECONDS); + Thread actualCallbackThread = callbackThreadFuture.get(timeoutMs, MILLISECONDS); + assertTrue(actualCallbackDelay.compareTo(duration) >= 0); + if (duration.isZero()) { + assertSame(Thread.currentThread(), actualCallbackThread); + } else { + assertTrue(actualCallbackDelay.compareTo(duration.multipliedBy(2)) < 0); + assertNotSame(Thread.currentThread(), actualCallbackThread); + } + } + + @ParameterizedTest + @ValueSource(longs = {0, SLEEP_DURATION_MILLIS}) + void closeBeforeSleepAsync(final long durationMs) { + Duration duration = Duration.ofMillis(durationMs); + try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); + DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { + assertAll( + () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByExecutorService, duration, backedByExecutorService::close), + () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByScheduledExecutorService, duration, backedByScheduledExecutorService::close) + ); + } + } + + /** + * {@link AsyncClientExecutor#close()} and {@link com.mongodb.connection.NettyTransportSettings.Builder#eventLoopGroup(EventLoopGroup)} + * forbit this scenario, but we still handle it. + */ + @Test + void backingExecutorShutdownBeforeSleepAsync() { + Duration duration = Duration.ofMillis(SLEEP_DURATION_MILLIS); + try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); + DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { + assertAll( + () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByExecutorService, duration, executorService::shutdownNow), + () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByScheduledExecutorService, duration, scheduledExecutorService::shutdownNow) + ); + } + } + + private static void assertCloseOrBackingExecutorShutdownBeforeSleepAsync( + final DefaultAsyncClientExecutor clientExecutor, final Duration duration, final Runnable doBeforeSleepAsync) throws Exception { + doBeforeSleepAsync.run(); + AtomicInteger completionCount = new AtomicInteger(); + CompletableFuture callbackFuture = new CompletableFuture<>(); + clientExecutor.sleepAsync(duration, (result, t) -> { + completionCount.incrementAndGet(); + if (t != null) { + callbackFuture.completeExceptionally(t); + } else { + callbackFuture.complete(result); + } + }); + Throwable callbackException = assertThrows(CompletionException.class, () -> callbackFuture.getNow(null)).getCause(); + assertInstanceOf(RejectedExecutionException.class, callbackException); + Thread.sleep(duration.toMillis() * 2); + assertEquals(1, completionCount.get()); + } + + @Test + void closeWhileSleepingInSleepAsync() { + Duration duration = Duration.ofMillis(SLEEP_DURATION_MILLIS); + try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); + DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { + assertAll( + () -> assertCloseWhileSleepingInSleepAsync(backedByExecutorService, duration), + () -> assertCloseWhileSleepingInSleepAsync(backedByScheduledExecutorService, duration) + ); + } + } + + private static void assertCloseWhileSleepingInSleepAsync( + final DefaultAsyncClientExecutor clientExecutor, final Duration duration) throws Exception { + AtomicInteger completionCount = new AtomicInteger(); + CompletableFuture callbackFuture = new CompletableFuture<>(); + clientExecutor.sleepAsync(duration, (result, t) -> { + completionCount.incrementAndGet(); + if (t != null) { + callbackFuture.completeExceptionally(t); + } else { + callbackFuture.complete(result); + } + }); + clientExecutor.close(); + long timeoutMs = duration.toMillis() * 2; + Throwable callbackException = assertThrows(ExecutionException.class, () -> callbackFuture.get(timeoutMs, MILLISECONDS)).getCause(); + assertInstanceOf(RejectedExecutionException.class, callbackException); + Thread.sleep(timeoutMs); + assertEquals(1, completionCount.get()); + } + + @Test + void closeWhileCallbackIsBeingCompletedInSleepAsync() { + Duration duration = Duration.ofMillis(SLEEP_DURATION_MILLIS); + try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); + DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { + assertAll( + () -> assertCloseWhileCallbackIsBeingCompletedInSleepAsync(backedByExecutorService, duration), + () -> assertCloseWhileCallbackIsBeingCompletedInSleepAsync(backedByScheduledExecutorService, duration) + ); + } + } + + private static void assertCloseWhileCallbackIsBeingCompletedInSleepAsync( + final DefaultAsyncClientExecutor clientExecutor, final Duration duration) throws Exception { + AtomicInteger completionCount = new AtomicInteger(); + CompletableFuture callbackFuture = new CompletableFuture<>(); + CompletableFuture closeFuture = new CompletableFuture<>(); + clientExecutor.sleepAsync(duration, (result, t) -> { + completionCount.incrementAndGet(); + if (t != null) { + callbackFuture.completeExceptionally(t); + } else { + callbackFuture.complete(result); + } + waitForCompletion(closeFuture, duration); + }); + waitForCompletion(callbackFuture, duration.multipliedBy(2)); + clientExecutor.close(); + closeFuture.complete(null); + long timeoutMs = duration.toMillis() * 2; + assertDoesNotThrow(() -> callbackFuture.get(timeoutMs, MILLISECONDS)); + Thread.sleep(timeoutMs); + assertEquals(1, completionCount.get()); + } + + private static void waitForCompletion(final Future future, final Duration duration) { + try { + future.get(duration.toNanos(), NANOSECONDS); + } catch (InterruptedException e) { + throw interruptAndCreateMongoInterruptedException(null, e); + } catch (TimeoutException e) { + throw new RuntimeException(e); + } catch (ExecutionException e) { + // nothing to do + } + } +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java new file mode 100644 index 00000000000..6308b77b701 --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.concurrent.Callable; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +class MongoScheduledThreadPoolExecutorTest extends MongoThreadPoolExecutorTest { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + @Override + void delegateErrorToDefaultUncaughtExceptionHandlerAndLog(final boolean taskCompletesAbruptlyWithError) throws Exception { + MongoScheduledThreadPoolExecutor executor = new MongoScheduledThreadPoolExecutor(1, new DaemonThreadFactory("test"), getLogger()); + try { + Error error = new AssertionError(); + RuntimeException exception = new RuntimeException(); + Throwable expectedException = taskCompletesAbruptlyWithError ? error : exception; + Runnable runnable = () -> { + if (taskCompletesAbruptlyWithError) { + throw error; + } else { + throw exception; + } + }; + Callable callable = () -> { + runnable.run(); + return null; + }; + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.execute(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.submit(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.submit(runnable, null)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.submit(callable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.schedule(runnable, 0, MILLISECONDS)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.schedule(callable, 0, MILLISECONDS)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.scheduleAtFixedRate(runnable, 0, 1, MILLISECONDS)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.scheduleWithFixedDelay(runnable, 0, 1, MILLISECONDS)); + } finally { + executor.shutdownNow(); + } + } +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java new file mode 100644 index 00000000000..5615c34887d --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java @@ -0,0 +1,125 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.internal.diagnostics.logging.Logger; +import com.mongodb.internal.diagnostics.logging.Loggers; +import com.mongodb.lang.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.lang.Thread.UncaughtExceptionHandler; +import java.time.Duration; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ScheduledThreadPoolExecutor; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.matches; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +class MongoThreadPoolExecutorTest { + private static final long TIMEOUT_MILLIS = 100; + + private Logger logger; + + @Nullable + private UncaughtExceptionHandler originalUncaughtExceptionHandler; + + @BeforeEach + void beforeEach() { + logger = spy(Loggers.getLogger("test")); + originalUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler(); + } + + @AfterEach + void afterEach() { + if (originalUncaughtExceptionHandler != null) { + Thread.setDefaultUncaughtExceptionHandler(originalUncaughtExceptionHandler); + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void delegateErrorToDefaultUncaughtExceptionHandlerAndLog(final boolean taskCompletesAbruptlyWithError) throws Exception { + MongoThreadPoolExecutor executor = new MongoThreadPoolExecutor( + 1, 1, Duration.ofMillis(TIMEOUT_MILLIS), new LinkedBlockingQueue<>(), new DaemonThreadFactory("test"), logger); + try { + Error error = new AssertionError(); + RuntimeException exception = new RuntimeException(); + Throwable expectedThrowable = taskCompletesAbruptlyWithError ? error : exception; + Runnable runnable = () -> { + if (taskCompletesAbruptlyWithError) { + throw error; + } else { + throw exception; + } + }; + Callable callable = () -> { + runnable.run(); + return null; + }; + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, true, () -> executor.execute(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, false, () -> executor.submit(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, false, () -> executor.submit(runnable, null)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, false, () -> executor.submit(callable)); + } finally { + executor.shutdownNow(); + } + } + + /** + * @param delegatesExceptionToUncaughtExceptionHandler The {@link ExecutorService#execute(Runnable)} method usually results in + * the task exception to be thrown from the worker thread {@link Thread#run()} method, + * though {@link ScheduledThreadPoolExecutor#execute(Runnable)} is an example where that is not the case. + * This is different from the behavior of the {@link ExecutorService#submit(Runnable)} method, and similar methods that return + * a {@link Future} holding the task exception. + * This parameter reflects the expected behavior, depending on the method used by {@code submitThrowingTask}. + */ + void assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog( + final Throwable expectedThrowable, + final boolean delegatesExceptionToUncaughtExceptionHandler, + final Runnable submitThrowingTask) throws Exception { + CompletableFuture uncaughtExceptionFuture = new CompletableFuture<>(); + Thread.setDefaultUncaughtExceptionHandler((t, e) -> { + uncaughtExceptionFuture.complete(e); + }); + clearInvocations(logger); + submitThrowingTask.run(); + Thread.sleep(TIMEOUT_MILLIS); + if (expectedThrowable instanceof Error || delegatesExceptionToUncaughtExceptionHandler) { + Throwable actualUncaughtException = uncaughtExceptionFuture.get(TIMEOUT_MILLIS, MILLISECONDS); + assertSame(expectedThrowable, actualUncaughtException); + } else { + assertFalse(uncaughtExceptionFuture.isDone()); + } + verify(logger).error(matches("A task completed abruptly"), any()); + } + + Logger getLogger() { + return logger; + } +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/UnimplementedAsyncClientExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/UnimplementedAsyncClientExecutorTest.java new file mode 100644 index 00000000000..632e92a29d7 --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/UnimplementedAsyncClientExecutorTest.java @@ -0,0 +1,59 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.time.Duration; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class UnimplementedAsyncClientExecutorTest { + @ParameterizedTest + @ValueSource(longs = {0, 200}) + void sleepAsync(final long durationMs) { + Duration duration = Duration.ofMillis(durationMs); + assertSleepAsync(duration); + } + + private static void assertSleepAsync(final Duration duration) { + CompletableFuture callbackExceptionFuture = new CompletableFuture<>(); + CompletableFuture callbackThreadFuture = new CompletableFuture<>(); + try (UnimplementedAsyncClientExecutor unimplementedClientExecutor = UnimplementedAsyncClientExecutor.instance()) { + unimplementedClientExecutor.sleepAsync(duration, (result, t) -> { + if (t != null) { + callbackExceptionFuture.completeExceptionally(t); + } else { + callbackExceptionFuture.complete(result); + } + callbackThreadFuture.complete(Thread.currentThread()); + }); + } + if (duration.isZero()) { + assertDoesNotThrow(() -> callbackExceptionFuture.getNow(null)); + } else { + Throwable callbackException = assertThrows(CompletionException.class, () -> callbackExceptionFuture.getNow(null)).getCause(); + assertSame(AssertionError.class, callbackException.getClass()); + } + Thread actualCallbackThread = callbackThreadFuture.getNow(null); + assertSame(Thread.currentThread(), actualCallbackThread); + } +} diff --git a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java index 9ba2139f18c..bb8e31c7d2d 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java @@ -36,15 +36,10 @@ import com.mongodb.client.model.bulk.ClientBulkWriteResult; import com.mongodb.client.model.bulk.ClientNamespacedWriteModel; import com.mongodb.connection.ClusterDescription; -import com.mongodb.connection.SocketSettings; import com.mongodb.internal.TimeoutSettings; import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.connection.ClientMetadata; import com.mongodb.internal.connection.Cluster; -import com.mongodb.internal.connection.DefaultClusterFactory; -import com.mongodb.internal.connection.InternalConnectionPoolSettings; -import com.mongodb.internal.connection.StreamFactory; -import com.mongodb.internal.connection.StreamFactoryFactory; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; import com.mongodb.internal.session.ServerSessionPool; @@ -61,7 +56,6 @@ import static com.mongodb.assertions.Assertions.notNull; import static com.mongodb.client.internal.Crypts.createCrypt; -import static com.mongodb.internal.event.EventListenerHelper.getCommandListener; import static java.lang.String.format; import static org.bson.codecs.configuration.CodecRegistries.withUuidRepresentation; @@ -167,6 +161,7 @@ public ReadConcern getReadConcern() { } @Override + @Nullable public Long getTimeout(final TimeUnit timeUnit) { return delegate.getTimeout(timeUnit); } @@ -310,27 +305,6 @@ public ClientBulkWriteResult bulkWrite( return delegate.bulkWrite(clientSession, clientWriteModels, options); } - private static Cluster createCluster(final MongoClientSettings settings, - @Nullable final MongoDriverInformation mongoDriverInformation, - final StreamFactory streamFactory, final StreamFactory heartbeatStreamFactory) { - notNull("settings", settings); - return new DefaultClusterFactory().createCluster(settings.getClusterSettings(), settings.getServerSettings(), - settings.getConnectionPoolSettings(), InternalConnectionPoolSettings.builder().build(), - TimeoutSettings.create(settings), streamFactory, - TimeoutSettings.createHeartbeatSettings(settings), heartbeatStreamFactory, - settings.getCredential(), settings.getLoggerSettings(), getCommandListener(settings.getCommandListeners()), - settings.getApplicationName(), mongoDriverInformation, settings.getCompressorList(), settings.getServerApi(), - settings.getDnsClient()); - } - - private static StreamFactory getStreamFactory( - final StreamFactoryFactory streamFactoryFactory, - final MongoClientSettings settings, - final boolean isHeartbeat) { - SocketSettings socketSettings = isHeartbeat ? settings.getHeartbeatSocketSettings() : settings.getSocketSettings(); - return streamFactoryFactory.create(socketSettings, settings.getSslSettings()); - } - public Cluster getCluster() { return delegate.getCluster(); } From 76809460155edbce6221d902700c70c5ce5451a6 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Fri, 26 Jun 2026 15:47:43 -0600 Subject: [PATCH 02/14] Thread `AsyncClientExecutor` through to make it accessible JAVA-6240 --- .../mongodb/internal/async/AsyncRunnable.java | 2 + .../RetryingAsyncCallbackSupplier.java | 9 +++- .../async/function/RetryingSyncSupplier.java | 3 +- .../connection/DefaultClusterFactory.java | 6 ++- .../InternalOperationContextFactory.java | 10 +++- .../internal/connection/OperationContext.java | 52 +++++++++++-------- .../operation/AsyncOperationHelper.java | 2 +- .../internal/session/ServerSessionPool.java | 18 +++++-- .../com/mongodb/ClusterFixture.java | 7 +-- .../TlsChannelStreamFunctionalTest.java | 3 +- .../RetryingAsyncCallbackSupplierTest.java | 26 ++++++++++ ...mConnectionInitializerSpecification.groovy | 2 +- .../ScramShaAuthenticatorSpecification.groovy | 2 +- .../ServerSelectionSelectionTest.java | 2 + .../ServerSessionPoolSpecification.groovy | 13 +++-- .../src/main/com/mongodb/MongoClient.java | 3 +- .../mongodb/MongoClientSpecification.groovy | 10 +++- .../reactivestreams/client/MongoClients.java | 8 +-- .../client/internal/MongoClientImpl.java | 40 ++++++++------ .../internal/OperationExecutorImpl.java | 1 + .../internal/crypt/KeyManagementService.java | 3 +- .../client/internal/MongoClientImplTest.java | 11 ++-- .../com/mongodb/client/internal/Clusters.java | 2 +- .../client/internal/MongoClientImpl.java | 38 +++++++++----- .../client/internal/MongoClusterImpl.java | 19 ++++--- .../com/mongodb/client/MongoClientTest.java | 17 +++--- .../client/MongoClientSpecification.groovy | 20 ++++--- .../internal/MongoClusterSpecification.groovy | 3 +- 28 files changed, 229 insertions(+), 103 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java b/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java index f5fb6358b87..0a1b40b0a20 100644 --- a/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java +++ b/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java @@ -20,6 +20,7 @@ import com.mongodb.internal.async.function.LoopControl; import com.mongodb.internal.async.function.RetryControl; import com.mongodb.internal.async.function.RetryingAsyncCallbackSupplier; +import com.mongodb.internal.thread.AsyncClientExecutor; import java.util.function.BooleanSupplier; import java.util.function.Predicate; @@ -233,6 +234,7 @@ default AsyncSupplier thenSupply(final AsyncSupplier supplier) { default AsyncRunnable thenRunRetryingWhile(final AsyncRunnable runnable, final Predicate shouldRetry) { return thenRun(callback -> { new RetryingAsyncCallbackSupplier( + AsyncClientExecutor.unimplemented(), new RetryControl<>(new SimpleRetryPolicy(shouldRetry)), // `finish` is required here instead of `unsafeFinish` // because only `finish` meets the contract of diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java index abdfefe55e4..eabf51955f5 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java @@ -17,6 +17,7 @@ import com.mongodb.annotations.NotThreadSafe; import com.mongodb.internal.async.SingleResultCallback; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import static com.mongodb.assertions.Assertions.assertNotNull; @@ -34,14 +35,20 @@ */ @NotThreadSafe public final class RetryingAsyncCallbackSupplier implements AsyncCallbackSupplier { + private final AsyncClientExecutor clientExecutor; private final RetryControl control; private final AsyncCallbackSupplier asyncFunction; /** + * @param clientExecutor VAKOTODO document * @param control The {@link RetryControl} to control the new {@link RetryingAsyncCallbackSupplier}. * @param asyncFunction The retryable {@link AsyncCallbackSupplier} to be decorated. */ - public RetryingAsyncCallbackSupplier(final RetryControl control, final AsyncCallbackSupplier asyncFunction) { + public RetryingAsyncCallbackSupplier( + final AsyncClientExecutor clientExecutor, + final RetryControl control, + final AsyncCallbackSupplier asyncFunction) { + this.clientExecutor = clientExecutor; this.control = control; this.asyncFunction = asyncFunction; } diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java index 3af14b46786..5fc53b9facf 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java @@ -16,6 +16,7 @@ package com.mongodb.internal.async.function; import com.mongodb.annotations.NotThreadSafe; +import com.mongodb.internal.thread.AsyncClientExecutor; import java.util.function.Supplier; @@ -37,7 +38,7 @@ public final class RetryingSyncSupplier implements Supplier { private final Supplier syncFunction; /** - * See {@link RetryingAsyncCallbackSupplier#RetryingAsyncCallbackSupplier(RetryControl, AsyncCallbackSupplier)}. + * See {@link RetryingAsyncCallbackSupplier#RetryingAsyncCallbackSupplier(AsyncClientExecutor, RetryControl, AsyncCallbackSupplier)}. */ public RetryingSyncSupplier(final RetryControl control, final Supplier syncFunction) { this.control = control; diff --git a/driver-core/src/main/com/mongodb/internal/connection/DefaultClusterFactory.java b/driver-core/src/main/com/mongodb/internal/connection/DefaultClusterFactory.java index ac853cb002e..668af7119c3 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/DefaultClusterFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/DefaultClusterFactory.java @@ -35,6 +35,7 @@ import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.DnsClient; @@ -65,6 +66,7 @@ public Cluster createCluster(final ClusterSettings originalClusterSettings, fina final StreamFactory streamFactory, final TimeoutSettings heartbeatTimeoutSettings, final StreamFactory heartbeatStreamFactory, + final AsyncClientExecutor clientExecutor, @Nullable final MongoCredential credential, final LoggerSettings loggerSettings, @Nullable final CommandListener commandListener, @@ -103,9 +105,9 @@ public Cluster createCluster(final ClusterSettings originalClusterSettings, fina DnsSrvRecordMonitorFactory dnsSrvRecordMonitorFactory = new DefaultDnsSrvRecordMonitorFactory(clusterId, serverSettings, dnsClient); InternalOperationContextFactory clusterOperationContextFactory = - new InternalOperationContextFactory(clusterTimeoutSettings, serverApi); + new InternalOperationContextFactory(clusterTimeoutSettings, serverApi, clientExecutor); InternalOperationContextFactory heartBeatOperationContextFactory = - new InternalOperationContextFactory(heartbeatTimeoutSettings, serverApi); + new InternalOperationContextFactory(heartbeatTimeoutSettings, serverApi, clientExecutor); ClientMetadata clientMetadata = new ClientMetadata( applicationName, diff --git a/driver-core/src/main/com/mongodb/internal/connection/InternalOperationContextFactory.java b/driver-core/src/main/com/mongodb/internal/connection/InternalOperationContextFactory.java index 4653c90050b..851ca5a38ca 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/InternalOperationContextFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/InternalOperationContextFactory.java @@ -18,6 +18,7 @@ import com.mongodb.ServerApi; import com.mongodb.internal.TimeoutContext; import com.mongodb.internal.TimeoutSettings; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import static com.mongodb.internal.connection.OperationContext.simpleOperationContext; @@ -27,17 +28,22 @@ public final class InternalOperationContextFactory { private final TimeoutSettings timeoutSettings; @Nullable private final ServerApi serverApi; + private final AsyncClientExecutor clientExecutor; - public InternalOperationContextFactory(final TimeoutSettings timeoutSettings, @Nullable final ServerApi serverApi) { + public InternalOperationContextFactory( + final TimeoutSettings timeoutSettings, + @Nullable final ServerApi serverApi, + final AsyncClientExecutor clientExecutor) { this.timeoutSettings = timeoutSettings; this.serverApi = serverApi; + this.clientExecutor = clientExecutor; } /** * @return a simple operation context without timeoutMS */ OperationContext create() { - return simpleOperationContext(timeoutSettings.connectionOnly(), serverApi); + return simpleOperationContext(timeoutSettings.connectionOnly(), serverApi, clientExecutor); } /** diff --git a/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java b/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java index a4f772eccce..c8ed571ab23 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java +++ b/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java @@ -28,9 +28,11 @@ import com.mongodb.internal.IgnorableRequestContext; import com.mongodb.internal.TimeoutContext; import com.mongodb.internal.TimeoutSettings; +import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.observability.micrometer.Span; import com.mongodb.internal.observability.micrometer.TracingManager; import com.mongodb.internal.session.SessionContext; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.selector.ServerSelector; @@ -41,6 +43,7 @@ import java.util.concurrent.atomic.AtomicLong; import static com.mongodb.MongoException.SYSTEM_OVERLOADED_ERROR_LABEL; +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; import static java.util.stream.Collectors.toList; /** @@ -58,19 +61,23 @@ public class OperationContext { private final ServerApi serverApi; @Nullable private final String operationName; + private final AsyncClientExecutor clientExecutor; @Nullable private Span tracingSpan; + @VisibleForTesting(otherwise = PRIVATE) public OperationContext(final RequestContext requestContext, final SessionContext sessionContext, final TimeoutContext timeoutContext, @Nullable final ServerApi serverApi) { - this(requestContext, sessionContext, timeoutContext, TracingManager.NO_OP, serverApi, null); + this(requestContext, sessionContext, timeoutContext, AsyncClientExecutor.unimplemented(), TracingManager.NO_OP, serverApi, null); } public OperationContext(final RequestContext requestContext, final SessionContext sessionContext, final TimeoutContext timeoutContext, + final AsyncClientExecutor clientExecutor, final TracingManager tracingManager, @Nullable final ServerApi serverApi, @Nullable final String operationName) { this(NEXT_ID.incrementAndGet(), requestContext, sessionContext, timeoutContext, new ServerDeprioritization(), + clientExecutor, tracingManager, serverApi, operationName, @@ -78,52 +85,49 @@ public OperationContext(final RequestContext requestContext, final SessionContex } public OperationContext(final RequestContext requestContext, final SessionContext sessionContext, final TimeoutContext timeoutContext, + final AsyncClientExecutor clientExecutor, final TracingManager tracingManager, @Nullable final ServerApi serverApi, @Nullable final String operationName, final ServerDeprioritization serverDeprioritization) { this(NEXT_ID.incrementAndGet(), requestContext, sessionContext, timeoutContext, serverDeprioritization, + clientExecutor, tracingManager, serverApi, operationName, null); } - static OperationContext simpleOperationContext( - final TimeoutSettings timeoutSettings, @Nullable final ServerApi serverApi) { + public static OperationContext simpleOperationContext( + final TimeoutSettings timeoutSettings, @Nullable final ServerApi serverApi, final AsyncClientExecutor clientExecutor) { return new OperationContext( IgnorableRequestContext.INSTANCE, NoOpSessionContext.INSTANCE, new TimeoutContext(timeoutSettings), + clientExecutor, TracingManager.NO_OP, serverApi, - null - ); + null); } - public static OperationContext simpleOperationContext(final TimeoutContext timeoutContext) { - return new OperationContext( - IgnorableRequestContext.INSTANCE, - NoOpSessionContext.INSTANCE, - timeoutContext, - TracingManager.NO_OP, - null, - null); + @VisibleForTesting(otherwise = PRIVATE) + static OperationContext simpleOperationContext(final TimeoutSettings timeoutSettings) { + return simpleOperationContext(timeoutSettings, null, AsyncClientExecutor.unimplemented()); } public OperationContext withSessionContext(final SessionContext sessionContext) { - return new OperationContext(id, requestContext, sessionContext, timeoutContext, serverDeprioritization, tracingManager, serverApi, - operationName, tracingSpan); + return new OperationContext(id, requestContext, sessionContext, timeoutContext, serverDeprioritization, clientExecutor, + tracingManager, serverApi, operationName, tracingSpan); } public OperationContext withTimeoutContext(final TimeoutContext timeoutContext) { - return new OperationContext(id, requestContext, sessionContext, timeoutContext, serverDeprioritization, tracingManager, serverApi, - operationName, tracingSpan); + return new OperationContext(id, requestContext, sessionContext, timeoutContext, serverDeprioritization, clientExecutor, + tracingManager, serverApi, operationName, tracingSpan); } public OperationContext withOperationName(final String operationName) { - return new OperationContext(id, requestContext, sessionContext, timeoutContext, serverDeprioritization, tracingManager, serverApi, - operationName, tracingSpan); + return new OperationContext(id, requestContext, sessionContext, timeoutContext, serverDeprioritization, clientExecutor, + tracingManager, serverApi, operationName, tracingSpan); } /** @@ -132,8 +136,8 @@ public OperationContext withOperationName(final String operationName) { */ public OperationContext withNewServerDeprioritization() { return new OperationContext(id, requestContext, sessionContext, timeoutContext, - new ServerDeprioritization(serverDeprioritization.enableOverloadRetargeting), tracingManager, serverApi, - operationName, tracingSpan); + new ServerDeprioritization(serverDeprioritization.enableOverloadRetargeting), clientExecutor, + tracingManager, serverApi, operationName, tracingSpan); } public long getId() { @@ -166,6 +170,10 @@ public String getOperationName() { return operationName; } + public AsyncClientExecutor getClientExecutor() { + return clientExecutor; + } + @Nullable public Span getTracingSpan() { return tracingSpan; @@ -180,6 +188,7 @@ private OperationContext(final long id, final SessionContext sessionContext, final TimeoutContext timeoutContext, final ServerDeprioritization serverDeprioritization, + final AsyncClientExecutor clientExecutor, final TracingManager tracingManager, @Nullable final ServerApi serverApi, @Nullable final String operationName, @@ -190,6 +199,7 @@ private OperationContext(final long id, this.requestContext = requestContext; this.sessionContext = sessionContext; this.timeoutContext = timeoutContext; + this.clientExecutor = clientExecutor; this.tracingManager = tracingManager; this.serverApi = serverApi; this.operationName = operationName; diff --git a/driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java b/driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java index 9bed9d84a6e..83c2055d014 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java +++ b/driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java @@ -327,7 +327,7 @@ static AsyncCallbackSupplier decorateWithRetriesAsync( final RetryControl retryControl, final OperationContext operationContext, final AsyncCallbackSupplier supplier) { - return new RetryingAsyncCallbackSupplier<>(retryControl, callback -> { + return new RetryingAsyncCallbackSupplier<>(operationContext.getClientExecutor(), retryControl, callback -> { retryControl.getPolicy().onAttemptStart(retryControl, operationContext); supplier.get(callback); }); diff --git a/driver-core/src/main/com/mongodb/internal/session/ServerSessionPool.java b/driver-core/src/main/com/mongodb/internal/session/ServerSessionPool.java index 9111eaed3a9..df301021308 100644 --- a/driver-core/src/main/com/mongodb/internal/session/ServerSessionPool.java +++ b/driver-core/src/main/com/mongodb/internal/session/ServerSessionPool.java @@ -24,11 +24,14 @@ import com.mongodb.internal.IgnorableRequestContext; import com.mongodb.internal.TimeoutContext; import com.mongodb.internal.TimeoutSettings; +import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.connection.Cluster; import com.mongodb.internal.connection.Connection; import com.mongodb.internal.connection.NoOpSessionContext; import com.mongodb.internal.connection.OperationContext; +import com.mongodb.internal.observability.micrometer.TracingManager; import com.mongodb.internal.selector.ReadPreferenceServerSelector; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.internal.validator.NoOpFieldNameValidator; import com.mongodb.lang.Nullable; import com.mongodb.selector.ServerSelector; @@ -50,6 +53,7 @@ import java.util.concurrent.atomic.LongAdder; import static com.mongodb.assertions.Assertions.isTrue; +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; import static java.util.concurrent.TimeUnit.MINUTES; /** @@ -67,17 +71,23 @@ interface Clock { long millis(); } - public ServerSessionPool(final Cluster cluster, final TimeoutSettings timeoutSettings, @Nullable final ServerApi serverApi) { + public ServerSessionPool( + final Cluster cluster, + final AsyncClientExecutor clientExecutor, + final TimeoutSettings timeoutSettings, + @Nullable final ServerApi serverApi) { this(cluster, new OperationContext(IgnorableRequestContext.INSTANCE, NoOpSessionContext.INSTANCE, - new TimeoutContext(timeoutSettings.connectionOnly()), serverApi)); + new TimeoutContext(timeoutSettings.connectionOnly()), clientExecutor, TracingManager.NO_OP, serverApi, null)); } - public ServerSessionPool(final Cluster cluster, final OperationContext operationContext) { + @VisibleForTesting(otherwise = PRIVATE) + ServerSessionPool(final Cluster cluster, final OperationContext operationContext) { this(cluster, operationContext, System::currentTimeMillis); } - public ServerSessionPool(final Cluster cluster, final OperationContext operationContext, final Clock clock) { + @VisibleForTesting(otherwise = PRIVATE) + ServerSessionPool(final Cluster cluster, final OperationContext operationContext, final Clock clock) { this.cluster = cluster; this.operationContext = operationContext; this.clock = clock; diff --git a/driver-core/src/test/functional/com/mongodb/ClusterFixture.java b/driver-core/src/test/functional/com/mongodb/ClusterFixture.java index bfe41263b3c..b2019a62e29 100644 --- a/driver-core/src/test/functional/com/mongodb/ClusterFixture.java +++ b/driver-core/src/test/functional/com/mongodb/ClusterFixture.java @@ -66,6 +66,7 @@ import com.mongodb.internal.operation.DropDatabaseOperation; import com.mongodb.internal.operation.ReadOperation; import com.mongodb.internal.operation.WriteOperation; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslContextBuilder; @@ -205,7 +206,7 @@ public static OperationContext createOperationContext() { } public static final InternalOperationContextFactory OPERATION_CONTEXT_FACTORY = - new InternalOperationContextFactory(TIMEOUT_SETTINGS, getServerApi()); + new InternalOperationContextFactory(TIMEOUT_SETTINGS, getServerApi(), AsyncClientExecutor.unimplemented()); public static OperationContext createOperationContext(final TimeoutSettings timeoutSettings) { return new OperationContext( @@ -449,7 +450,7 @@ private static Cluster createCluster(final MongoCredential credential, final Str return new DefaultClusterFactory().createCluster(ClusterSettings.builder().hosts(asList(getPrimary())).build(), ServerSettings.builder().build(), ConnectionPoolSettings.builder().maxSize(1).build(), InternalConnectionPoolSettings.builder().build(), - TIMEOUT_SETTINGS.connectionOnly(), streamFactory, TIMEOUT_SETTINGS.connectionOnly(), streamFactory, credential, + TIMEOUT_SETTINGS.connectionOnly(), streamFactory, TIMEOUT_SETTINGS.connectionOnly(), streamFactory, AsyncClientExecutor.unimplemented(), credential, LoggerSettings.builder().build(), null, null, null, Collections.emptyList(), getServerApi(), null); } @@ -461,7 +462,7 @@ private static Cluster createCluster(final ConnectionString connectionString, fi InternalConnectionPoolSettings.builder().build(), TimeoutSettings.create(mongoClientSettings).connectionOnly(), streamFactory, TimeoutSettings.createHeartbeatSettings(mongoClientSettings).connectionOnly(), new SocketStreamFactory(new DefaultInetAddressResolver(), SocketSettings.builder().readTimeout(5, SECONDS).build(), - getSslSettings(connectionString)), + getSslSettings(connectionString)), AsyncClientExecutor.unimplemented(), connectionString.getCredential(), LoggerSettings.builder().build(), null, null, null, connectionString.getCompressorList(), getServerApi(), null); diff --git a/driver-core/src/test/functional/com/mongodb/internal/connection/TlsChannelStreamFunctionalTest.java b/driver-core/src/test/functional/com/mongodb/internal/connection/TlsChannelStreamFunctionalTest.java index 46e0cc6f964..e7ae01c0879 100644 --- a/driver-core/src/test/functional/com/mongodb/internal/connection/TlsChannelStreamFunctionalTest.java +++ b/driver-core/src/test/functional/com/mongodb/internal/connection/TlsChannelStreamFunctionalTest.java @@ -21,7 +21,6 @@ import com.mongodb.ServerAddress; import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; -import com.mongodb.internal.TimeoutContext; import com.mongodb.internal.TimeoutSettings; import org.bson.ByteBuf; import org.bson.ByteBufNIO; @@ -162,7 +161,7 @@ public T answer(final InvocationOnMock invocationOnMock) throws Throwable { } private static OperationContext createOperationContext(final int connectTimeoutMs) { - return simpleOperationContext(new TimeoutContext(TimeoutSettings.DEFAULT.withConnectTimeoutMS(connectTimeoutMs))); + return simpleOperationContext(TimeoutSettings.DEFAULT.withConnectTimeoutMS(connectTimeoutMs)); } @Test diff --git a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java index 9a9ecad1bcf..aa8a011bb0f 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java @@ -16,19 +16,42 @@ package com.mongodb.internal.async.function; import com.mongodb.internal.async.function.RetryingSyncSupplierTest.AssertingUnusedRetryPolicy; +import com.mongodb.internal.thread.AsyncClientExecutor; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + import static com.mongodb.internal.async.AsyncRunnable.beginAsync; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; final class RetryingAsyncCallbackSupplierTest { + private ExecutorService executorService; + private AsyncClientExecutor clientExecutor; + + @BeforeEach + void beforeEach() { + executorService = Executors.newSingleThreadScheduledExecutor(); + clientExecutor = AsyncClientExecutor.backedBy(executorService); + } + + @AfterEach + void afterEach() { + if (executorService != null) { + executorService.shutdownNow(); + } + } + @Test void doWhileDisabledThrowsAtFirstAttempt() { RetryControl retryControl = new RetryControl<>(new AssertingUnusedRetryPolicy(false)); RuntimeException exception = new RuntimeException(); RetryingAsyncCallbackSupplier retryingSupplier = new RetryingAsyncCallbackSupplier<>( + clientExecutor, retryControl, callback -> { retryControl.doWhileDisabledAsync(actionCallback -> { @@ -44,6 +67,7 @@ void doWhileDisabledThrowsAtSecondAttempt() { RetryControl retryControl = new RetryControl<>(new AssertingUnusedRetryPolicy(true)); RuntimeException exception = new RuntimeException(); RetryingAsyncCallbackSupplier retryingSupplier = new RetryingAsyncCallbackSupplier<>( + clientExecutor, retryControl, callback -> { if (retryControl.isFirstAttempt()) { @@ -63,6 +87,7 @@ void doWhileDisabledCompletesNormally() { RetryControl retryControl = new RetryControl<>(new AssertingUnusedRetryPolicy(true)); Object result = new Object(); RetryingAsyncCallbackSupplier retryingSupplier = new RetryingAsyncCallbackSupplier<>( + clientExecutor, retryControl, callback -> { beginAsync().thenSupply(c -> { @@ -84,6 +109,7 @@ void doWhileDisabledNestedThrowsAtFirstAttempt() { RetryControl retryControl = new RetryControl<>(new AssertingUnusedRetryPolicy(false)); RuntimeException exception = new RuntimeException(); RetryingAsyncCallbackSupplier retryingSupplier = new RetryingAsyncCallbackSupplier<>( + clientExecutor, retryControl, callback -> { retryControl.doWhileDisabledAsync(actionCallback -> { diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionInitializerSpecification.groovy b/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionInitializerSpecification.groovy index a7bfaa36cce..d3f05ea8b49 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionInitializerSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/InternalStreamConnectionInitializerSpecification.groovy @@ -53,7 +53,7 @@ class InternalStreamConnectionInitializerSpecification extends Specification { def serverId = new ServerId(new ClusterId(), new ServerAddress()) def internalConnection = new TestInternalConnection(serverId, ServerType.STANDALONE) - def operationContext = simpleOperationContext(TimeoutSettings.DEFAULT, null) + def operationContext = simpleOperationContext(TimeoutSettings.DEFAULT) def 'should create correct description'() { given: diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ScramShaAuthenticatorSpecification.groovy b/driver-core/src/test/unit/com/mongodb/internal/connection/ScramShaAuthenticatorSpecification.groovy index 21f9bc28161..ed7d758c21b 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ScramShaAuthenticatorSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ScramShaAuthenticatorSpecification.groovy @@ -41,7 +41,7 @@ import static org.junit.Assert.assertEquals class ScramShaAuthenticatorSpecification extends Specification { def serverId = new ServerId(new ClusterId(), new ServerAddress('localhost', 27017)) def connectionDescription = new ConnectionDescription(serverId) - def operationContext = simpleOperationContext(TimeoutSettings.DEFAULT, null) + def operationContext = simpleOperationContext(TimeoutSettings.DEFAULT) private final static MongoCredentialWithCache SHA1_CREDENTIAL = new MongoCredentialWithCache(createScramSha1Credential('user', 'database', 'pencil' as char[])) private final static MongoCredentialWithCache SHA256_CREDENTIAL = diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java index 5d6b5e0e1e3..1baf2d14053 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java @@ -41,6 +41,7 @@ import com.mongodb.internal.observability.micrometer.TracingManager; import com.mongodb.internal.selector.ReadPreferenceServerSelector; import com.mongodb.internal.selector.WritableServerSelector; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.internal.time.Timeout; import com.mongodb.lang.NonNull; import com.mongodb.lang.Nullable; @@ -303,6 +304,7 @@ private OperationContext createOperationContext() { IgnorableRequestContext.INSTANCE, NoOpSessionContext.INSTANCE, new TimeoutContext(TIMEOUT_SETTINGS.withServerSelectionTimeoutMS(0)), + AsyncClientExecutor.unimplemented(), TracingManager.NO_OP, null, null, diff --git a/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy b/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy index 0fc4564d322..2a740caea71 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy @@ -25,6 +25,7 @@ import com.mongodb.internal.connection.Cluster import com.mongodb.internal.connection.Connection import com.mongodb.internal.connection.Server import com.mongodb.internal.connection.ServerTuple +import com.mongodb.internal.thread.AsyncClientExecutor import com.mongodb.internal.validator.NoOpFieldNameValidator import org.bson.BsonArray import org.bson.BsonBinarySubType @@ -71,7 +72,7 @@ class ServerSessionPoolSpecification extends Specification { def cluster = Stub(Cluster) { getCurrentDescription() >> connectedDescription } - def pool = new ServerSessionPool(cluster, TIMEOUT_SETTINGS, getServerApi()) + def pool = createServerSessionPool(cluster) when: def session = pool.get() @@ -85,7 +86,7 @@ class ServerSessionPoolSpecification extends Specification { def cluster = Stub(Cluster) { getCurrentDescription() >> connectedDescription } - def pool = new ServerSessionPool(cluster, TIMEOUT_SETTINGS, getServerApi()) + def pool = createServerSessionPool(cluster) pool.close() when: @@ -100,7 +101,7 @@ class ServerSessionPoolSpecification extends Specification { def cluster = Stub(Cluster) { getCurrentDescription() >> connectedDescription } - def pool = new ServerSessionPool(cluster, TIMEOUT_SETTINGS, getServerApi()) + def pool = createServerSessionPool(cluster) def session = pool.get() when: @@ -207,7 +208,7 @@ class ServerSessionPoolSpecification extends Specification { def cluster = Mock(Cluster) { getCurrentDescription() >> connectedDescription } - def pool = new ServerSessionPool(cluster, TIMEOUT_SETTINGS, getServerApi()) + def pool = createServerSessionPool(cluster) def sessions = [] 10.times { sessions.add(pool.get()) } @@ -226,4 +227,8 @@ class ServerSessionPoolSpecification extends Specification { { it instanceof BsonDocumentCodec }, _) >> new BsonDocument() 1 * connection.release() } + + static createServerSessionPool(Cluster cluster) { + new ServerSessionPool(cluster, AsyncClientExecutor.unimplemented(), TIMEOUT_SETTINGS, getServerApi()) + } } diff --git a/driver-legacy/src/main/com/mongodb/MongoClient.java b/driver-legacy/src/main/com/mongodb/MongoClient.java index 06dac49c671..de1d7e641bc 100644 --- a/driver-legacy/src/main/com/mongodb/MongoClient.java +++ b/driver-legacy/src/main/com/mongodb/MongoClient.java @@ -41,6 +41,7 @@ import com.mongodb.internal.connection.StreamFactoryFactory; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; +import com.mongodb.internal.observability.micrometer.TracingManager; import com.mongodb.internal.session.ServerSessionPool; import com.mongodb.internal.thread.DaemonThreadFactory; import com.mongodb.internal.validator.NoOpFieldNameValidator; @@ -860,7 +861,7 @@ private void cleanCursors() { ServerCursorAndNamespace cur; while ((cur = orphanedCursors.poll()) != null) { OperationContext operationContext = new OperationContext(IgnorableRequestContext.INSTANCE, NoOpSessionContext.INSTANCE, - new TimeoutContext(getTimeoutSettings()), options.getServerApi()); + new TimeoutContext(getTimeoutSettings()), delegate.getClientExecutor(), TracingManager.NO_OP, options.getServerApi(), null); ReadWriteBinding binding = new SingleServerBinding(delegate.getCluster(), cur.serverCursor.getAddress()); try { diff --git a/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy b/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy index 1389a41c760..d7a36d5ea5a 100644 --- a/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy +++ b/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy @@ -23,6 +23,8 @@ import com.mongodb.client.model.geojson.MultiPolygon import com.mongodb.connection.ClusterSettings import com.mongodb.internal.connection.ClientMetadata import com.mongodb.internal.connection.Cluster +import com.mongodb.internal.connection.StreamFactoryFactory +import com.mongodb.internal.thread.AsyncClientExecutor import org.bson.BsonDocument import org.bson.Document import org.bson.codecs.UuidCodec @@ -314,7 +316,7 @@ class MongoClientSpecification extends Specification { def clusterStub = Stub(Cluster) clusterStub.getClientMetadata() >> new ClientMetadata("test", MongoDriverInformation.builder().build()) - def client = new MongoClientImpl(clusterStub, null, MongoClientSettings.builder().build(), null, executor) + def client = new MongoClientImpl(clusterStub, null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), executor) when: client.watch((Class) null) @@ -365,4 +367,10 @@ class MongoClientSpecification extends Specification { cleanup: client?.close() } + + def mockStreamFactoryFactory() { + Mock(StreamFactoryFactory) { + getClientExecutor() >> AsyncClientExecutor.unimplemented() + } + } } diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java index 57ee076039e..da722d16ba0 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java @@ -27,6 +27,7 @@ import com.mongodb.internal.connection.InternalConnectionPoolSettings; import com.mongodb.internal.connection.StreamFactory; import com.mongodb.internal.connection.StreamFactoryFactory; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.reactivestreams.client.internal.MongoClientImpl; import com.mongodb.spi.dns.InetAddressResolver; @@ -118,7 +119,7 @@ public static MongoClient create(final MongoClientSettings settings, @Nullable f StreamFactory streamFactory = getStreamFactory(streamFactoryFactory, settings, false); StreamFactory heartbeatStreamFactory = getStreamFactory(streamFactoryFactory, settings, true); MongoDriverInformation wrappedMongoDriverInformation = wrapMongoDriverInformation(mongoDriverInformation); - Cluster cluster = createCluster(settings, wrappedMongoDriverInformation, streamFactory, heartbeatStreamFactory); + Cluster cluster = createCluster(settings, wrappedMongoDriverInformation, streamFactory, heartbeatStreamFactory, streamFactoryFactory.getClientExecutor()); return new MongoClientImpl(settings, wrappedMongoDriverInformation, cluster, streamFactoryFactory); } @@ -135,12 +136,13 @@ public static CodecRegistry getDefaultCodecRegistry() { private static Cluster createCluster(final MongoClientSettings settings, @Nullable final MongoDriverInformation mongoDriverInformation, - final StreamFactory streamFactory, final StreamFactory heartbeatStreamFactory) { + final StreamFactory streamFactory, final StreamFactory heartbeatStreamFactory, + final AsyncClientExecutor clientExecutor) { notNull("settings", settings); return new DefaultClusterFactory().createCluster(settings.getClusterSettings(), settings.getServerSettings(), settings.getConnectionPoolSettings(), InternalConnectionPoolSettings.builder().prestartAsyncWorkManager(true).build(), TimeoutSettings.create(settings), streamFactory, TimeoutSettings.createHeartbeatSettings(settings), heartbeatStreamFactory, - settings.getCredential(), settings.getLoggerSettings(), getCommandListener(settings.getCommandListeners()), + clientExecutor, settings.getCredential(), settings.getLoggerSettings(), getCommandListener(settings.getCommandListeners()), settings.getApplicationName(), mongoDriverInformation, settings.getCompressorList(), settings.getServerApi(), settings.getDnsClient()); } diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java index 8fda2e9294d..dbcc27f5022 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java @@ -29,12 +29,15 @@ import com.mongodb.client.model.bulk.ClientNamespacedWriteModel; import com.mongodb.connection.ClusterDescription; import com.mongodb.internal.TimeoutSettings; +import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.connection.ClientMetadata; import com.mongodb.internal.connection.Cluster; +import com.mongodb.internal.connection.StreamFactoryFactory; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; import com.mongodb.internal.observability.micrometer.TracingManager; import com.mongodb.internal.session.ServerSessionPool; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.reactivestreams.client.ChangeStreamPublisher; import com.mongodb.reactivestreams.client.ClientSession; @@ -56,6 +59,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import static com.mongodb.assertions.Assertions.notNull; +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; import static java.lang.String.format; import static org.bson.codecs.configuration.CodecRegistries.withUuidRepresentation; @@ -69,29 +73,30 @@ public final class MongoClientImpl implements MongoClient { private static final Logger LOGGER = Loggers.getLogger("client"); private final MongoClientSettings settings; - private final AutoCloseable externalResourceCloser; + private final StreamFactoryFactory streamFactoryFactory; private final MongoClusterImpl delegate; private final AtomicBoolean closed; public MongoClientImpl(final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, final Cluster cluster, - @Nullable final AutoCloseable externalResourceCloser) { - this(settings, mongoDriverInformation, cluster, null, externalResourceCloser); + final StreamFactoryFactory streamFactoryFactory) { + this(settings, mongoDriverInformation, cluster, null, streamFactoryFactory); } - public MongoClientImpl(final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, final Cluster cluster, - @Nullable final OperationExecutor executor) { - this(settings, mongoDriverInformation, cluster, executor, null); + @VisibleForTesting(otherwise = PRIVATE) + MongoClientImpl(final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, final Cluster cluster, + final StreamFactoryFactory streamFactoryFactory, @Nullable final OperationExecutor executor) { + this(settings, mongoDriverInformation, cluster, executor, streamFactoryFactory); } private MongoClientImpl(final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, final Cluster cluster, - @Nullable final OperationExecutor executor, @Nullable final AutoCloseable externalResourceCloser) { + @Nullable final OperationExecutor executor, final StreamFactoryFactory streamFactoryFactory) { notNull("settings", settings); notNull("cluster", cluster); TracingManager tracingManager = new TracingManager(settings.getObservabilitySettings()); TimeoutSettings timeoutSettings = TimeoutSettings.create(settings); - ServerSessionPool serverSessionPool = new ServerSessionPool(cluster, timeoutSettings, settings.getServerApi()); + ServerSessionPool serverSessionPool = new ServerSessionPool(cluster, streamFactoryFactory.getClientExecutor(), timeoutSettings, settings.getServerApi()); ClientSessionHelper clientSessionHelper = new ClientSessionHelper(this, serverSessionPool, tracingManager); AutoEncryptionSettings autoEncryptSettings = settings.getAutoEncryptionSettings(); @@ -117,7 +122,7 @@ private MongoClientImpl(final MongoClientSettings settings, final MongoDriverInf this.delegate = new MongoClusterImpl(cluster, crypt, operationExecutor, serverSessionPool, clientSessionHelper, mongoOperationPublisher); - this.externalResourceCloser = externalResourceCloser; + this.streamFactoryFactory = streamFactoryFactory; this.settings = settings; this.closed = new AtomicBoolean(); @@ -155,12 +160,10 @@ public void close() { } getServerSessionPool().close(); getCluster().close(); - if (externalResourceCloser != null) { - try { - externalResourceCloser.close(); - } catch (Exception e) { - LOGGER.warn("Exception closing resource", e); - } + try { + streamFactoryFactory.close(); + } catch (Exception e) { + LOGGER.warn("Exception closing resource", e); } } } @@ -336,4 +339,11 @@ public void appendMetadata(final MongoDriverInformation mongoDriverInformation) clientMetadata.append(mongoDriverInformation); LOGGER.info(format("MongoClient metadata has been updated to %s", clientMetadata.getBsonDocument())); } + + /** + * @see StreamFactoryFactory#getClientExecutor() + */ + public AsyncClientExecutor getClientExecutor() { + return streamFactoryFactory.getClientExecutor(); + } } diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/OperationExecutorImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/OperationExecutorImpl.java index 5e11e0503a7..fa1956c69a1 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/OperationExecutorImpl.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/OperationExecutorImpl.java @@ -243,6 +243,7 @@ private OperationContext getOperationContext(final RequestContext requestContext requestContext, new ReadConcernAwareNoOpSessionContext(readConcern), createTimeoutContext(session, timeoutSettings), + mongoClient.getClientExecutor(), tracingManager, mongoClient.getSettings().getServerApi(), commandName, diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java index 67ebf421c9c..ae3cf36f392 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java @@ -36,6 +36,7 @@ import com.mongodb.internal.crypt.capi.MongoKeyDecryptor; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.internal.time.Timeout; import com.mongodb.lang.Nullable; import org.bson.ByteBuf; @@ -182,7 +183,7 @@ private OperationContext createOperationContext(@Nullable final Timeout operatio throw new MongoOperationTimeoutException(TIMEOUT_ERROR_MESSAGE); }); } - return OperationContext.simpleOperationContext(new TimeoutContext(timeoutSettings)); + return OperationContext.simpleOperationContext(timeoutSettings, null, AsyncClientExecutor.unimplemented()); } @NonNull diff --git a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java index 0fda131f4ff..e28410d1331 100644 --- a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java +++ b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java @@ -24,9 +24,11 @@ import com.mongodb.internal.client.model.changestream.ChangeStreamLevel; import com.mongodb.internal.connection.ClientMetadata; import com.mongodb.internal.connection.Cluster; +import com.mongodb.internal.connection.StreamFactoryFactory; import com.mongodb.internal.mockito.MongoMockito; import com.mongodb.internal.observability.micrometer.TracingManager; import com.mongodb.internal.session.ServerSessionPool; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.reactivestreams.client.ChangeStreamPublisher; import com.mongodb.reactivestreams.client.ClientSession; import com.mongodb.reactivestreams.client.ListDatabasesPublisher; @@ -204,11 +206,14 @@ void testStartSession() { private MongoClientImpl createMongoClient() { MongoDriverInformation mongoDriverInformation = MongoDriverInformation.builder().driverName("reactive-streams").build(); - Cluster mock = MongoMockito.mock(Cluster.class, cluster -> { - when(cluster.getClientMetadata()) + Cluster cluster = MongoMockito.mock(Cluster.class, mock -> { + when(mock.getClientMetadata()) .thenReturn(new ClientMetadata("test", mongoDriverInformation)); }); + StreamFactoryFactory streamFactoryFactory = MongoMockito.mock(StreamFactoryFactory.class, mock -> { + when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.unimplemented()); + }); return new MongoClientImpl(MongoClientSettings.builder().build(), - mongoDriverInformation, mock, OPERATION_EXECUTOR); + mongoDriverInformation, cluster, streamFactoryFactory, OPERATION_EXECUTOR); } } diff --git a/driver-sync/src/main/com/mongodb/client/internal/Clusters.java b/driver-sync/src/main/com/mongodb/client/internal/Clusters.java index 6c57505e090..ffedcfbace7 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/Clusters.java +++ b/driver-sync/src/main/com/mongodb/client/internal/Clusters.java @@ -47,7 +47,7 @@ public static Cluster createCluster(final MongoClientSettings settings, return new DefaultClusterFactory().createCluster(settings.getClusterSettings(), settings.getServerSettings(), settings.getConnectionPoolSettings(), InternalConnectionPoolSettings.builder().build(), TimeoutSettings.create(settings), streamFactory, - TimeoutSettings.createHeartbeatSettings(settings), heartbeatStreamFactory, + TimeoutSettings.createHeartbeatSettings(settings), heartbeatStreamFactory, streamFactoryFactory.getClientExecutor(), settings.getCredential(), settings.getLoggerSettings(), getCommandListener(settings.getCommandListeners()), settings.getApplicationName(), mongoDriverInformation, settings.getCompressorList(), settings.getServerApi(), settings.getDnsClient()); diff --git a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java index bb8e31c7d2d..2cd7ffffa5d 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java @@ -40,10 +40,12 @@ import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.connection.ClientMetadata; import com.mongodb.internal.connection.Cluster; +import com.mongodb.internal.connection.StreamFactoryFactory; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; import com.mongodb.internal.session.ServerSessionPool; import com.mongodb.internal.observability.micrometer.TracingManager; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import org.bson.BsonDocument; import org.bson.Document; @@ -56,6 +58,7 @@ import static com.mongodb.assertions.Assertions.notNull; import static com.mongodb.client.internal.Crypts.createCrypt; +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; import static java.lang.String.format; import static org.bson.codecs.configuration.CodecRegistries.withUuidRepresentation; @@ -69,23 +72,23 @@ public final class MongoClientImpl implements MongoClient { private final MongoDriverInformation mongoDriverInformation; private final MongoClusterImpl delegate; private final AtomicBoolean closed; - private final AutoCloseable externalResourceCloser; + private final StreamFactoryFactory streamFactoryFactory; public MongoClientImpl(final Cluster cluster, final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, - @Nullable final AutoCloseable externalResourceCloser) { - this(cluster, mongoDriverInformation, settings, externalResourceCloser, null); + final StreamFactoryFactory streamFactoryFactory) { + this(cluster, mongoDriverInformation, settings, streamFactoryFactory, null); } - @VisibleForTesting(otherwise = VisibleForTesting.AccessModifier.PRIVATE) + @VisibleForTesting(otherwise = PRIVATE) public MongoClientImpl(final Cluster cluster, final MongoDriverInformation mongoDriverInformation, final MongoClientSettings settings, - @Nullable final AutoCloseable externalResourceCloser, + final StreamFactoryFactory streamFactoryFactory, @Nullable final OperationExecutor operationExecutor) { - this.externalResourceCloser = externalResourceCloser; + this.streamFactoryFactory = streamFactoryFactory; this.settings = notNull("settings", settings); this.mongoDriverInformation = mongoDriverInformation; AutoEncryptionSettings autoEncryptionSettings = settings.getAutoEncryptionSettings(); @@ -94,15 +97,17 @@ public MongoClientImpl(final Cluster cluster, + SynchronousContextProvider.class.getName() + " when using the synchronous driver"); } + AsyncClientExecutor clientExecutor = streamFactoryFactory.getClientExecutor(); this.delegate = new MongoClusterImpl(autoEncryptionSettings, cluster, withUuidRepresentation(settings.getCodecRegistry(), settings.getUuidRepresentation()), (SynchronousContextProvider) settings.getContextProvider(), autoEncryptionSettings == null ? null : createCrypt(settings, autoEncryptionSettings), this, operationExecutor, settings.getReadConcern(), settings.getReadPreference(), settings.getRetryReads(), settings.getRetryWrites(), settings.getEnableOverloadRetargeting(), settings.getServerApi(), - new ServerSessionPool(cluster, TimeoutSettings.create(settings), settings.getServerApi()), + new ServerSessionPool(cluster, clientExecutor, TimeoutSettings.create(settings), settings.getServerApi()), TimeoutSettings.create(settings), settings.getUuidRepresentation(), - settings.getWriteConcern(), new TracingManager(settings.getObservabilitySettings())); + settings.getWriteConcern(), clientExecutor, + new TracingManager(settings.getObservabilitySettings())); this.closed = new AtomicBoolean(); BsonDocument clientMetadataDocument = delegate.getCluster().getClientMetadata().getBsonDocument(); @@ -118,12 +123,10 @@ public void close() { } delegate.getServerSessionPool().close(); delegate.getCluster().close(); - if (externalResourceCloser != null) { - try { - externalResourceCloser.close(); - } catch (Exception e) { - LOGGER.warn("Exception closing resource", e); - } + try { + streamFactoryFactory.close(); + } catch (Exception e) { + LOGGER.warn("Exception closing resource", e); } } } @@ -328,4 +331,11 @@ public MongoClientSettings getSettings() { public MongoDriverInformation getMongoDriverInformation() { return mongoDriverInformation; } + + /** + * @see StreamFactoryFactory#getClientExecutor() + */ + public AsyncClientExecutor getClientExecutor() { + return streamFactoryFactory.getClientExecutor(); + } } diff --git a/driver-sync/src/main/com/mongodb/client/internal/MongoClusterImpl.java b/driver-sync/src/main/com/mongodb/client/internal/MongoClusterImpl.java index 5d3ac6bc678..9ed66d84f1d 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/MongoClusterImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/MongoClusterImpl.java @@ -59,6 +59,7 @@ import com.mongodb.internal.operation.ReadOperation; import com.mongodb.internal.operation.WriteOperation; import com.mongodb.internal.session.ServerSessionPool; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import org.bson.BsonDocument; import org.bson.Document; @@ -102,6 +103,7 @@ final class MongoClusterImpl implements MongoCluster { private final UuidRepresentation uuidRepresentation; private final WriteConcern writeConcern; private final Operations operations; + private final AsyncClientExecutor clientExecutor; private final TracingManager tracingManager; MongoClusterImpl( @@ -110,7 +112,8 @@ final class MongoClusterImpl implements MongoCluster { @Nullable final OperationExecutor operationExecutor, final ReadConcern readConcern, final ReadPreference readPreference, final boolean retryReads, final boolean retryWrites, final boolean enableOverloadRetargeting, @Nullable final ServerApi serverApi, final ServerSessionPool serverSessionPool, final TimeoutSettings timeoutSettings, - final UuidRepresentation uuidRepresentation, final WriteConcern writeConcern, final TracingManager tracingManager) { + final UuidRepresentation uuidRepresentation, final WriteConcern writeConcern, + final AsyncClientExecutor clientExecutor, final TracingManager tracingManager) { this.autoEncryptionSettings = autoEncryptionSettings; this.cluster = cluster; this.codecRegistry = codecRegistry; @@ -128,6 +131,7 @@ final class MongoClusterImpl implements MongoCluster { this.timeoutSettings = timeoutSettings; this.uuidRepresentation = uuidRepresentation; this.writeConcern = writeConcern; + this.clientExecutor = clientExecutor; this.tracingManager = tracingManager; operations = new Operations<>( null, @@ -172,35 +176,35 @@ public Long getTimeout(final TimeUnit timeUnit) { public MongoCluster withCodecRegistry(final CodecRegistry codecRegistry) { return new MongoClusterImpl(autoEncryptionSettings, cluster, codecRegistry, contextProvider, crypt, originator, operationExecutor, readConcern, readPreference, retryReads, retryWrites, enableOverloadRetargeting, serverApi, serverSessionPool, timeoutSettings, - uuidRepresentation, writeConcern, tracingManager); + uuidRepresentation, writeConcern, clientExecutor, tracingManager); } @Override public MongoCluster withReadPreference(final ReadPreference readPreference) { return new MongoClusterImpl(autoEncryptionSettings, cluster, codecRegistry, contextProvider, crypt, originator, operationExecutor, readConcern, readPreference, retryReads, retryWrites, enableOverloadRetargeting, serverApi, serverSessionPool, timeoutSettings, - uuidRepresentation, writeConcern, tracingManager); + uuidRepresentation, writeConcern, clientExecutor, tracingManager); } @Override public MongoCluster withWriteConcern(final WriteConcern writeConcern) { return new MongoClusterImpl(autoEncryptionSettings, cluster, codecRegistry, contextProvider, crypt, originator, operationExecutor, readConcern, readPreference, retryReads, retryWrites, enableOverloadRetargeting, serverApi, serverSessionPool, timeoutSettings, - uuidRepresentation, writeConcern, tracingManager); + uuidRepresentation, writeConcern, clientExecutor, tracingManager); } @Override public MongoCluster withReadConcern(final ReadConcern readConcern) { return new MongoClusterImpl(autoEncryptionSettings, cluster, codecRegistry, contextProvider, crypt, originator, operationExecutor, readConcern, readPreference, retryReads, retryWrites, enableOverloadRetargeting, serverApi, serverSessionPool, timeoutSettings, - uuidRepresentation, writeConcern, tracingManager); + uuidRepresentation, writeConcern, clientExecutor, tracingManager); } @Override public MongoCluster withTimeout(final long timeout, final TimeUnit timeUnit) { return new MongoClusterImpl(autoEncryptionSettings, cluster, codecRegistry, contextProvider, crypt, originator, operationExecutor, readConcern, readPreference, retryReads, retryWrites, enableOverloadRetargeting, serverApi, serverSessionPool, - timeoutSettings.withTimeout(timeout, timeUnit), uuidRepresentation, writeConcern, tracingManager); + timeoutSettings.withTimeout(timeout, timeUnit), uuidRepresentation, writeConcern, clientExecutor, tracingManager); } @Override @@ -397,7 +401,7 @@ private ClientBulkWriteResult executeBulkWrite( return operationExecutor.execute(operations.clientBulkWriteOperation(clientWriteModels, options), readConcern, clientSession); } - final class OperationExecutorImpl implements OperationExecutor { + private final class OperationExecutorImpl implements OperationExecutor { private final TimeoutSettings executorTimeoutSettings; OperationExecutorImpl(final TimeoutSettings executorTimeoutSettings) { @@ -526,6 +530,7 @@ private OperationContext getOperationContext(final ClientSession session, final getRequestContext(), new ReadConcernAwareNoOpSessionContext(readConcern), createTimeoutContext(session, executorTimeoutSettings), + clientExecutor, tracingManager, serverApi, commandName, diff --git a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java index 6d3413f032a..07e50f8dd77 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java @@ -25,7 +25,9 @@ import com.mongodb.event.ClusterOpeningEvent; import com.mongodb.internal.connection.ClientMetadata; import com.mongodb.internal.connection.Cluster; +import com.mongodb.internal.connection.StreamFactoryFactory; import com.mongodb.internal.mockito.MongoMockito; +import com.mongodb.internal.thread.AsyncClientExecutor; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -63,7 +65,7 @@ public void clusterOpening(final ClusterOpeningEvent event) { } @Test - void shouldCloseExternalResources() throws Exception { + void shouldCloseStreamFactoryFactory() { //given MongoDriverInformation mongoDriverInformation = MongoDriverInformation.builder().build(); @@ -74,11 +76,12 @@ void shouldCloseExternalResources() throws Exception { when(mockedCluster.getClientMetadata()) .thenReturn(new ClientMetadata("test", mongoDriverInformation)); }); - AutoCloseable externalResource = MongoMockito.mock( - AutoCloseable.class, - mockedExternalResource -> { + StreamFactoryFactory streamFactoryFactory = MongoMockito.mock( + StreamFactoryFactory.class, + mock -> { + when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.unimplemented()); try { - doNothing().when(mockedExternalResource).close(); + doNothing().when(mock).close(); } catch (Exception e) { throw new RuntimeException(e); } @@ -88,13 +91,13 @@ void shouldCloseExternalResources() throws Exception { cluster, MongoClientSettings.builder().build(), mongoDriverInformation, - externalResource); + streamFactoryFactory); //when mongoClient.close(); //then - Mockito.verify(externalResource).close(); + Mockito.verify(streamFactoryFactory).close(); Mockito.verify(cluster).close(); } } diff --git a/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy b/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy index 916d8179af5..22a06f89d98 100644 --- a/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy +++ b/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy @@ -37,6 +37,8 @@ import com.mongodb.internal.TimeoutSettings import com.mongodb.internal.client.model.changestream.ChangeStreamLevel import com.mongodb.internal.connection.ClientMetadata import com.mongodb.internal.connection.Cluster +import com.mongodb.internal.connection.StreamFactoryFactory +import com.mongodb.internal.thread.AsyncClientExecutor import org.bson.BsonDocument import org.bson.Document import org.bson.codecs.UuidCodec @@ -70,7 +72,7 @@ class MongoClientSpecification extends Specification { .retryWrites(true) .codecRegistry(CODEC_REGISTRY) .build() - def client = new MongoClientImpl(Stub(Cluster), null, settings, null, new TestOperationExecutor([])) + def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), new TestOperationExecutor([])) when: def database = client.getDatabase('name') @@ -87,7 +89,7 @@ class MongoClientSpecification extends Specification { def 'should use ListDatabasesIterableImpl correctly'() { given: def executor = new TestOperationExecutor([null, null]) - def client = new MongoClientImpl(Stub(Cluster), null, MongoClientSettings.builder().build(), null, executor) + def client = new MongoClientImpl(Stub(Cluster), null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), executor) def listDatabasesMethod = client.&listDatabases def listDatabasesNamesMethod = client.&listDatabaseNames @@ -132,7 +134,7 @@ class MongoClientSpecification extends Specification { .build() def readPreference = settings.getReadPreference() def readConcern = settings.getReadConcern() - def client = new MongoClientImpl(Stub(Cluster), null, settings, null, executor) + def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), executor) def watchMethod = client.&watch when: @@ -169,7 +171,7 @@ class MongoClientSpecification extends Specification { def 'should validate the ChangeStreamIterable pipeline data correctly'() { given: def executor = new TestOperationExecutor([]) - def client = new MongoClientImpl(Stub(Cluster), null, MongoClientSettings.builder().build(), null, + def client = new MongoClientImpl(Stub(Cluster), null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), executor) when: @@ -201,7 +203,7 @@ class MongoClientSpecification extends Specification { 1 * getClientMetadata() >> new ClientMetadata("test", driverInformation) } def settings = MongoClientSettings.builder().build() - def client = new MongoClientImpl(cluster, driverInformation, settings, null, new TestOperationExecutor([])) + def client = new MongoClientImpl(cluster, driverInformation, settings, mockStreamFactoryFactory(), new TestOperationExecutor([])) expect: client.getClusterDescription() == clusterDescription @@ -216,7 +218,7 @@ class MongoClientSpecification extends Specification { .build() when: - def client = new MongoClientImpl(Stub(Cluster), null, settings, null, new TestOperationExecutor([])) + def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), new TestOperationExecutor([])) then: (client.getCodecRegistry().get(UUID) as UuidCodec).getUuidRepresentation() == C_SHARP_LEGACY @@ -224,4 +226,10 @@ class MongoClientSpecification extends Specification { cleanup: client?.close() } + + def mockStreamFactoryFactory() { + Mock(StreamFactoryFactory) { + getClientExecutor() >> AsyncClientExecutor.unimplemented() + } + } } diff --git a/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy b/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy index 34f46e7b007..db1937dcbd5 100644 --- a/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy +++ b/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy @@ -29,6 +29,7 @@ import com.mongodb.internal.client.model.changestream.ChangeStreamLevel import com.mongodb.internal.connection.Cluster import com.mongodb.internal.session.ServerSessionPool import com.mongodb.internal.observability.micrometer.TracingManager +import com.mongodb.internal.thread.AsyncClientExecutor import org.bson.BsonDocument import org.bson.Document import org.bson.codecs.UuidCodec @@ -260,6 +261,6 @@ class MongoClusterSpecification extends Specification { new MongoClusterImpl(null, cluster, settings.codecRegistry, null, null, originator, operationExecutor, settings.readConcern, settings.readPreference, settings.retryReads, settings.retryWrites, settings.enableOverloadRetargeting, null, serverSessionPool, TimeoutSettings.create(settings), settings.uuidRepresentation, - settings.writeConcern, TracingManager.NO_OP) + settings.writeConcern, AsyncClientExecutor.unimplemented(), TracingManager.NO_OP) } } From 8951205c71edd4abc273a3ae3eea8a6a5f81c05c Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Thu, 25 Jun 2026 07:45:48 -0600 Subject: [PATCH 03/14] Add backoff to `RetryAttemptInfo` JAVA-6240 --- .../mongodb/internal/async/AsyncRunnable.java | 1 + .../internal/async/SimpleRetryPolicy.java | 6 +- .../internal/async/function/RetryPolicy.java | 19 +++++- .../RetryingAsyncCallbackSupplier.java | 59 ++++++++----------- .../async/function/RetryingSyncSupplier.java | 27 +++++++-- .../internal/operation/SpecRetryPolicy.java | 3 +- .../async/function/RetryControlTest.java | 29 +++++---- .../RetryingAsyncCallbackSupplierTest.java | 35 +++++++++++ .../function/RetryingSyncSupplierTest.java | 26 +++++++- 9 files changed, 149 insertions(+), 56 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java b/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java index 0a1b40b0a20..bf42ebb555e 100644 --- a/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java +++ b/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java @@ -234,6 +234,7 @@ default AsyncSupplier thenSupply(final AsyncSupplier supplier) { default AsyncRunnable thenRunRetryingWhile(final AsyncRunnable runnable, final Predicate shouldRetry) { return thenRun(callback -> { new RetryingAsyncCallbackSupplier( + // `AsyncClientExecutor` is not needed, given the contract of `SimpleRetryPolicy`, `RetryingAsyncCallbackSupplier` AsyncClientExecutor.unimplemented(), new RetryControl<>(new SimpleRetryPolicy(shouldRetry)), // `finish` is required here instead of `unsafeFinish` diff --git a/driver-core/src/main/com/mongodb/internal/async/SimpleRetryPolicy.java b/driver-core/src/main/com/mongodb/internal/async/SimpleRetryPolicy.java index ade2a70c119..3854c77a891 100644 --- a/driver-core/src/main/com/mongodb/internal/async/SimpleRetryPolicy.java +++ b/driver-core/src/main/com/mongodb/internal/async/SimpleRetryPolicy.java @@ -19,8 +19,12 @@ import com.mongodb.internal.async.function.RetryPolicy; import com.mongodb.internal.async.function.RetryPolicy.Decision.RetryAttemptInfo; +import java.time.Duration; import java.util.function.Predicate; +/** + * {@link RetryAttemptInfo#getBackoff()} is always {@link Duration#isZero() zero}. + */ final class SimpleRetryPolicy implements RetryPolicy { private final Predicate shouldRetry; @@ -30,6 +34,6 @@ final class SimpleRetryPolicy implements RetryPolicy { @Override public Decision onAttemptFailure(final RetryContext retryContext, final Throwable attemptFailedResult) { - return new Decision(attemptFailedResult, shouldRetry.test(attemptFailedResult) ? new RetryAttemptInfo() : null); + return new Decision(attemptFailedResult, shouldRetry.test(attemptFailedResult) ? new RetryAttemptInfo(Duration.ZERO) : null); } } diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryPolicy.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryPolicy.java index 6cbedb1ac2c..a84145b73e0 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryPolicy.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryPolicy.java @@ -18,9 +18,11 @@ import com.mongodb.annotations.NotThreadSafe; import com.mongodb.lang.Nullable; +import java.time.Duration; import java.util.Optional; import java.util.function.Supplier; +import static com.mongodb.assertions.Assertions.assertFalse; import static com.mongodb.assertions.Assertions.assertNotNull; /** @@ -87,12 +89,25 @@ public String toString() { * The information needed to start a retry attempt. */ public static final class RetryAttemptInfo { - public RetryAttemptInfo() { + private final Duration backoff; + + public RetryAttemptInfo(final Duration backoff) { + assertFalse(backoff.isNegative()); + this.backoff = backoff; + } + + /** + * A non-{@linkplain Duration#isNegative() negative} backoff. + */ + public Duration getBackoff() { + return backoff; } @Override public String toString() { - return "RetryAttemptInfo{}"; + return "RetryAttemptInfo{" + + "backoff=" + backoff + + '}'; } } } diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java index eabf51955f5..0a4c5761757 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java @@ -16,11 +16,14 @@ package com.mongodb.internal.async.function; import com.mongodb.annotations.NotThreadSafe; +import com.mongodb.internal.async.MutableValue; import com.mongodb.internal.async.SingleResultCallback; +import com.mongodb.internal.async.function.RetryPolicy.Decision.RetryAttemptInfo; import com.mongodb.internal.thread.AsyncClientExecutor; -import com.mongodb.lang.Nullable; -import static com.mongodb.assertions.Assertions.assertNotNull; +import java.time.Duration; + +import static com.mongodb.internal.async.AsyncRunnable.beginAsync; /** * A decorator that implements automatic retrying of failed executions of an {@link AsyncCallbackSupplier}. @@ -40,7 +43,8 @@ public final class RetryingAsyncCallbackSupplier implements AsyncCallbackSupp private final AsyncCallbackSupplier asyncFunction; /** - * @param clientExecutor VAKOTODO document + * @param clientExecutor For {@linkplain AsyncClientExecutor#sleepAsync(Duration, SingleResultCallback) delaying} attempts + * according to {@link RetryAttemptInfo#getBackoff()}. * @param control The {@link RetryControl} to control the new {@link RetryingAsyncCallbackSupplier}. * @param asyncFunction The retryable {@link AsyncCallbackSupplier} to be decorated. */ @@ -55,39 +59,24 @@ public RetryingAsyncCallbackSupplier( @Override public void get(final SingleResultCallback callback) { - // `asyncFunction` and `callback` are the only externally provided pieces of code for which we do not need to care about - // them throwing exceptions. If they do, that violates their contract and there is nothing we should do about it. - asyncFunction.get(new RetryingCallback(callback)); - } - - /** - * This callback is allowed to be completed more than once. - */ - @NotThreadSafe - private class RetryingCallback implements SingleResultCallback { - private final SingleResultCallback wrapped; - - RetryingCallback(final SingleResultCallback callback) { - wrapped = callback; - } - - @Override - public void onResult(@Nullable final R attemptSuccessfulResult, @Nullable final Throwable attemptFailedResult) { - if (attemptFailedResult != null) { + MutableValue> asyncFunctionSuccessfulResult = new MutableValue<>(); + beginAsync().thenRunWhileLoop(() -> asyncFunctionSuccessfulResult.getNullable() == null, iterationCallback -> { + beginAsync().thenSupply(asyncFunctionCallback -> { + asyncFunction.get(asyncFunctionCallback); + }).thenConsume((attemptSuccessfulResult, onAttemptSuccessCallback) -> { + // `attemptSuccessfulResult` may be `null`, so we have to wrap it in `MutableValue` for the while check to notice it + asyncFunctionSuccessfulResult.set(new MutableValue<>(attemptSuccessfulResult)); + onAttemptSuccessCallback.complete(onAttemptSuccessCallback); + }).onErrorIf(e -> true, (attemptFailedResult, onAttemptFailureCallback) -> { if (attemptFailedResult instanceof Error) { - wrapped.onResult(null, attemptFailedResult); - return; - } - try { - assertNotNull(control.advanceOrThrow(attemptFailedResult)); - } catch (Throwable retryingSupplierFailedResult) { - wrapped.onResult(null, retryingSupplierFailedResult); - return; + onAttemptFailureCallback.completeExceptionally(attemptFailedResult); + } else { + RetryAttemptInfo retryAttemptInfo = control.advanceOrThrow(attemptFailedResult); + clientExecutor.sleepAsync(retryAttemptInfo.getBackoff(), onAttemptFailureCallback); } - asyncFunction.get(this); - } else { - wrapped.onResult(attemptSuccessfulResult, null); - } - } + }).finish(iterationCallback); + }).thenSupply(c -> { + c.complete(asyncFunctionSuccessfulResult.get().getNullable()); + }).finish(callback); } } diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java index 5fc53b9facf..cbdfb709653 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java @@ -16,11 +16,16 @@ package com.mongodb.internal.async.function; import com.mongodb.annotations.NotThreadSafe; +import com.mongodb.internal.async.MutableValue; +import com.mongodb.internal.async.function.RetryPolicy.Decision.RetryAttemptInfo; import com.mongodb.internal.thread.AsyncClientExecutor; +import com.mongodb.lang.Nullable; +import java.time.Duration; import java.util.function.Supplier; -import static com.mongodb.assertions.Assertions.assertNotNull; +import static com.mongodb.internal.thread.InterruptionUtil.interruptAndCreateMongoInterruptedException; +import static java.util.concurrent.TimeUnit.NANOSECONDS; /** * A decorator that implements automatic retrying of failed executions of a {@link Supplier}. @@ -46,15 +51,29 @@ public RetryingSyncSupplier(final RetryControl control, final Supplier syn } @Override + @Nullable public R get() { - while (true) { + MutableValue> asyncFunctionSuccessfulResult = new MutableValue<>(); + while (asyncFunctionSuccessfulResult.getNullable() == null) { try { - return syncFunction.get(); + R attemptSuccessfulResult = syncFunction.get(); + // `attemptSuccessfulResult` may be `null`, so we have to wrap it in `MutableValue` for the while check to notice it + asyncFunctionSuccessfulResult.set(new MutableValue<>(attemptSuccessfulResult)); } catch (Error attemptFailedResult) { throw attemptFailedResult; } catch (Throwable attemptFailedResult) { - assertNotNull(control.advanceOrThrow(attemptFailedResult)); + RetryAttemptInfo retryAttemptInfo = control.advanceOrThrow(attemptFailedResult); + sleep(retryAttemptInfo.getBackoff()); } } + return asyncFunctionSuccessfulResult.get().getNullable(); + } + + private void sleep(final Duration duration) { + try { + NANOSECONDS.sleep(duration.toNanos()); + } catch (InterruptedException e) { + throw interruptAndCreateMongoInterruptedException(null, e); + } } } diff --git a/driver-core/src/main/com/mongodb/internal/operation/SpecRetryPolicy.java b/driver-core/src/main/com/mongodb/internal/operation/SpecRetryPolicy.java index 1615123cf53..417bb10a9ee 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/SpecRetryPolicy.java +++ b/driver-core/src/main/com/mongodb/internal/operation/SpecRetryPolicy.java @@ -31,6 +31,7 @@ import com.mongodb.internal.connection.OperationContext.ServerDeprioritization; import com.mongodb.lang.Nullable; +import java.time.Duration; import java.util.EnumMap; import java.util.EnumSet; import java.util.Set; @@ -176,7 +177,7 @@ public Decision onAttemptFailure(final RetryContext retryContext, final Throwabl boolean retry = retryableError && !maxAttemptsReached; Decision decision = new Decision( decideProspectiveFailedResult(retryContext.getProspectiveFailedResult().orElse(null), attemptFailedResult), - retry ? new RetryAttemptInfo() : null); + retry ? new RetryAttemptInfo(Duration.ZERO) : null); resetWriteRetryRequirementsInfo(); return decision; } diff --git a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryControlTest.java b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryControlTest.java index edf0f900401..71bba600689 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryControlTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryControlTest.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import java.time.Duration; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertAll; @@ -40,7 +41,7 @@ final class RetryControlTest { @Test void isFirstAttempt() { RetryControl retryControl = new RetryControl<>((retryContext, attemptFailedResult) -> - new Decision(attemptFailedResult, new RetryAttemptInfo())); + new Decision(attemptFailedResult, createRetryAttemptInfo())); assertTrue(retryControl.isFirstAttempt()); retryControl.advanceOrThrow(new RuntimeException()); assertFalse(retryControl.isFirstAttempt()); @@ -49,7 +50,7 @@ void isFirstAttempt() { @Test void attempt() { RetryControl retryControl = new RetryControl<>((retryContext, attemptFailedResult) -> - new Decision(attemptFailedResult, new RetryAttemptInfo())); + new Decision(attemptFailedResult, createRetryAttemptInfo())); assertEquals(0, retryControl.attempt()); retryControl.advanceOrThrow(new RuntimeException()); assertEquals(1, retryControl.attempt()); @@ -59,7 +60,7 @@ void attempt() { @Test void getPolicy() { - RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(attemptFailedResult, new RetryAttemptInfo()); + RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(attemptFailedResult, createRetryAttemptInfo()); RetryControl retryControl = new RetryControl<>(retryPolicy); assertSame(retryPolicy, retryControl.getPolicy()); } @@ -67,7 +68,7 @@ void getPolicy() { @Test void advanceOrThrowPassesCorrectArgumentsToAttemptFailure() { RetryPolicy retryPolicy = MongoMockito.mock(RetryPolicy.class, retryPolicyMock -> { - when(retryPolicyMock.onAttemptFailure(any(), any())).thenReturn(new Decision(new RuntimeException(), new RetryAttemptInfo())); + when(retryPolicyMock.onAttemptFailure(any(), any())).thenReturn(new Decision(new RuntimeException(), createRetryAttemptInfo())); }); RetryControl retryControl = new RetryControl<>(retryPolicy); RuntimeException attemptFailedResult = new RuntimeException(); @@ -84,7 +85,7 @@ void advanceOrThrowPassesCorrectArgumentsToAttemptFailure() { @Test void advanceOrThrowReturnsIfAnotherAttempt() { - RetryAttemptInfo immediateNextAttemptInfo = new RetryAttemptInfo(); + RetryAttemptInfo immediateNextAttemptInfo = createRetryAttemptInfo(); RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(attemptFailedResult, immediateNextAttemptInfo); RetryControl retryControl = new RetryControl<>(retryPolicy); RetryAttemptInfo actualImmediateNextAttemptInfo = retryControl.advanceOrThrow(new RuntimeException()); @@ -103,7 +104,7 @@ void advanceOrThrowThrowsIfNoMoreAttempts() { @Test void advanceOrThrowThrowsIfLastAttempt() { RuntimeException prospectiveFailedResult = new RuntimeException(); - RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(prospectiveFailedResult, new RetryAttemptInfo()); + RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(prospectiveFailedResult, createRetryAttemptInfo()); RetryControl retryControl = new RetryControl<>(retryPolicy); retryControl.advanceOrThrow(new RuntimeException()); assertThrows(prospectiveFailedResult.getClass(), () -> retryControl.breakAndThrowIfRetryAnd(() -> true)); @@ -114,7 +115,7 @@ void advanceOrThrowThrowsIfLastAttempt() { @Test void advanceOrThrowStoresProspectiveFailedResult() { RuntimeException prospectiveFailedResult = new RuntimeException(); - RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(prospectiveFailedResult, new RetryAttemptInfo()); + RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(prospectiveFailedResult, createRetryAttemptInfo()); RetryControl retryControl = new RetryControl<>(retryPolicy); retryControl.advanceOrThrow(new RuntimeException()); Optional actualProspectiveFailedResult = retryControl.getProspectiveFailedResult(); @@ -127,7 +128,7 @@ void advanceOrThrowStoresProspectiveFailedResult() { @Test void advanceOrThrowOverwritesProspectiveFailedResult() { - RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(attemptFailedResult, new RetryAttemptInfo()); + RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(attemptFailedResult, createRetryAttemptInfo()); RetryControl retryControl = new RetryControl<>(retryPolicy); retryControl.advanceOrThrow(new RuntimeException()); RuntimeException prospectiveFailedResult = new RuntimeException(); @@ -144,7 +145,7 @@ void advanceOrThrowOverwritesProspectiveFailedResult() { @DisplayName("breakAndThrowIfRetryAnd does nothing if first attempt") void breakAndThrowIfRetryAndDoesNothingIfFirstAttempt() { RetryControl retryControl = new RetryControl<>((retryContext, attemptFailedResult) -> - new Decision(attemptFailedResult, new RetryAttemptInfo())); + new Decision(attemptFailedResult, createRetryAttemptInfo())); assertDoesNotThrow(() -> retryControl.breakAndThrowIfRetryAnd(() -> true)); } @@ -152,7 +153,7 @@ void breakAndThrowIfRetryAndDoesNothingIfFirstAttempt() { @DisplayName("breakAndThrowIfRetryAnd throws if not first attempt") void breakAndThrowIfRetryAndThrowsIfNotFirstAttempt() { RuntimeException prospectiveFailedResult = new RuntimeException(); - RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(prospectiveFailedResult, new RetryAttemptInfo()); + RetryPolicy retryPolicy = (retryContext, attemptFailedResult) -> new Decision(prospectiveFailedResult, createRetryAttemptInfo()); RetryControl retryControl = new RetryControl<>(retryPolicy); retryControl.advanceOrThrow(new RuntimeException()); assertSame(prospectiveFailedResult, @@ -163,7 +164,7 @@ void breakAndThrowIfRetryAndThrowsIfNotFirstAttempt() { @DisplayName("breakAndThrowIfRetryAnd propagates if predicate throws") void breakAndThrowIfRetryAndPropagatesIfPredicateThrows() { RetryControl retryControl = new RetryControl<>((retryContext, attemptFailedResult) -> - new Decision(attemptFailedResult, new RetryAttemptInfo())); + new Decision(attemptFailedResult, createRetryAttemptInfo())); retryControl.advanceOrThrow(new RuntimeException()); RuntimeException predicateException = new RuntimeException(); assertSame(predicateException, @@ -178,7 +179,7 @@ void breakAndThrowIfRetryAndPropagatesIfPredicateThrows() { void breakAndThrowIfRetryAndAddsSuppressedProspectiveFailedResultIfPredicateThrows() { RuntimeException prospectiveFailedResult = new RuntimeException(); RetryControl retryControl = new RetryControl<>((retryContext, attemptFailedResult) -> - new Decision(prospectiveFailedResult, new RetryAttemptInfo())); + new Decision(prospectiveFailedResult, createRetryAttemptInfo())); retryControl.advanceOrThrow(new RuntimeException()); RuntimeException predicateException = new RuntimeException(); Throwable[] suppressed = assertThrows(predicateException.getClass(), @@ -190,4 +191,8 @@ void breakAndThrowIfRetryAndAddsSuppressedProspectiveFailedResultIfPredicateThro () -> assertSame(suppressed[0], prospectiveFailedResult) ); } + + private static RetryAttemptInfo createRetryAttemptInfo() { + return new RetryAttemptInfo(Duration.ZERO); + } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java index aa8a011bb0f..d21e0cf7a97 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplierTest.java @@ -17,17 +17,22 @@ import com.mongodb.internal.async.function.RetryingSyncSupplierTest.AssertingUnusedRetryPolicy; import com.mongodb.internal.thread.AsyncClientExecutor; +import com.mongodb.internal.time.StartTime; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.time.Duration; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import static com.mongodb.internal.async.AsyncRunnable.beginAsync; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; final class RetryingAsyncCallbackSupplierTest { private ExecutorService executorService; @@ -125,4 +130,34 @@ void doWhileDisabledNestedThrowsAtFirstAttempt() { retryingSupplier.get((r, t) -> assertSame(exception, t)); assertTrue(retryControl.isFirstAttempt()); } + + @Test + void backoff() throws Exception { + Duration backoff = Duration.ofMillis(400); + RetryControl retryControl = new RetryControl<>((retryContext, attemptFailedResult) -> + new RetryPolicy.Decision(attemptFailedResult, new RetryPolicy.Decision.RetryAttemptInfo(backoff))); + RetryingAsyncCallbackSupplier retryingSupplier = new RetryingAsyncCallbackSupplier<>( + AsyncClientExecutor.backedBy(executorService), + retryControl, + functionCallback -> { + beginAsync().thenRun(c -> { + if (retryControl.isFirstAttempt()) { + throw new RuntimeException(); + } + c.complete(c); + }).finish(functionCallback); + }); + StartTime startTime = StartTime.now(); + CompletableFuture durationFuture = new CompletableFuture<>(); + retryingSupplier.get((result, t) -> { + if (t != null) { + durationFuture.completeExceptionally(fail(t)); + } else { + durationFuture.complete(startTime.elapsed()); + } + }); + Duration duration = durationFuture.get(backoff.toMillis() * 2, MILLISECONDS); + assertTrue(duration.compareTo(backoff) >= 0); + assertTrue(duration.compareTo(backoff.multipliedBy(2)) < 0); + } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingSyncSupplierTest.java b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingSyncSupplierTest.java index f1ae3452171..090d8e10280 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingSyncSupplierTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/async/function/RetryingSyncSupplierTest.java @@ -15,9 +15,13 @@ */ package com.mongodb.internal.async.function; +import com.mongodb.internal.async.function.RetryPolicy.Decision; import com.mongodb.internal.async.function.RetryPolicy.Decision.RetryAttemptInfo; +import com.mongodb.internal.time.StartTime; import org.junit.jupiter.api.Test; +import java.time.Duration; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -96,6 +100,26 @@ void doWhileDisabledNestedThrowsAtFirstAttempt() { assertTrue(retryControl.isFirstAttempt()); } + @Test + void backoff() { + Duration backoff = Duration.ofMillis(50); + RetryControl retryControl = new RetryControl<>((retryContext, attemptFailedResult) -> + new Decision(attemptFailedResult, new RetryAttemptInfo(backoff))); + RetryingSyncSupplier retryingSupplier = new RetryingSyncSupplier<>( + retryControl, + () -> { + if (retryControl.isFirstAttempt()) { + throw new RuntimeException(); + } + return null; + }); + StartTime startTime = StartTime.now(); + retryingSupplier.get(); + Duration duration = startTime.elapsed(); + assertTrue(duration.compareTo(backoff) >= 0); + assertTrue(duration.compareTo(backoff.multipliedBy(2)) < 0); + } + static final class AssertingUnusedRetryPolicy implements RetryPolicy { private final boolean skipFailingOnFirstAttempt; @@ -106,7 +130,7 @@ static final class AssertingUnusedRetryPolicy implements RetryPolicy { @Override public Decision onAttemptFailure(final RetryContext retryContext, final Throwable attemptFailedResult) { if (skipFailingOnFirstAttempt && retryContext.isFirstAttempt()) { - return new Decision(attemptFailedResult, new RetryAttemptInfo()); + return new Decision(attemptFailedResult, new RetryAttemptInfo(Duration.ZERO)); } return fail(); } From 12602ecebaae945cc8009b1ebd5386fe7666cd71 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 28 Jul 2026 10:04:21 -0600 Subject: [PATCH 04/14] Make `RetryingSyncSupplier.sleep` `static` --- .../mongodb/internal/async/function/RetryingSyncSupplier.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java index cbdfb709653..124e6aa013e 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java @@ -69,7 +69,7 @@ public R get() { return asyncFunctionSuccessfulResult.get().getNullable(); } - private void sleep(final Duration duration) { + private static void sleep(final Duration duration) { try { NANOSECONDS.sleep(duration.toNanos()); } catch (InterruptedException e) { From 2546e07b4caebe12ef2e4b375b067b5f365c4e24 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 28 Jul 2026 11:17:55 -0600 Subject: [PATCH 05/14] Replace `AsyncClientExecutor.unimplemented` -> `NO_OP` --- .../main/com/mongodb/internal/async/AsyncRunnable.java | 2 +- .../com/mongodb/internal/connection/OperationContext.java | 4 ++-- .../mongodb/internal/connection/StreamFactoryHelper.java | 2 +- .../com/mongodb/internal/thread/AsyncClientExecutor.java | 8 +------- .../src/test/functional/com/mongodb/ClusterFixture.java | 6 +++--- .../internal/connection/ServerSelectionSelectionTest.java | 2 +- .../session/ServerSessionPoolSpecification.groovy | 2 +- .../test/unit/com/mongodb/MongoClientSpecification.groovy | 2 +- .../client/internal/crypt/KeyManagementService.java | 2 +- .../client/internal/MongoClientImplTest.java | 2 +- .../functional/com/mongodb/client/MongoClientTest.java | 2 +- .../com/mongodb/client/MongoClientSpecification.groovy | 2 +- .../client/internal/MongoClusterSpecification.groovy | 2 +- 13 files changed, 16 insertions(+), 22 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java b/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java index bf42ebb555e..37b55afe6af 100644 --- a/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java +++ b/driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java @@ -235,7 +235,7 @@ default AsyncRunnable thenRunRetryingWhile(final AsyncRunnable runnable, final P return thenRun(callback -> { new RetryingAsyncCallbackSupplier( // `AsyncClientExecutor` is not needed, given the contract of `SimpleRetryPolicy`, `RetryingAsyncCallbackSupplier` - AsyncClientExecutor.unimplemented(), + AsyncClientExecutor.NO_OP, new RetryControl<>(new SimpleRetryPolicy(shouldRetry)), // `finish` is required here instead of `unsafeFinish` // because only `finish` meets the contract of diff --git a/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java b/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java index 702a13d71f1..cb8d0830dd1 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java +++ b/driver-core/src/main/com/mongodb/internal/connection/OperationContext.java @@ -70,7 +70,7 @@ public class OperationContext { @VisibleForTesting(otherwise = PRIVATE) public OperationContext(final RequestContext requestContext, final SessionContext sessionContext, final TimeoutContext timeoutContext, @Nullable final ServerApi serverApi) { - this(requestContext, sessionContext, timeoutContext, AsyncClientExecutor.unimplemented(), TracingManager.NO_OP, serverApi, null); + this(requestContext, sessionContext, timeoutContext, AsyncClientExecutor.NO_OP, TracingManager.NO_OP, serverApi, null); } public OperationContext(final RequestContext requestContext, final SessionContext sessionContext, final TimeoutContext timeoutContext, @@ -114,7 +114,7 @@ public static OperationContext simpleOperationContext( @VisibleForTesting(otherwise = PRIVATE) static OperationContext simpleOperationContext(final TimeoutSettings timeoutSettings) { - return simpleOperationContext(timeoutSettings, null, AsyncClientExecutor.unimplemented()); + return simpleOperationContext(timeoutSettings, null, AsyncClientExecutor.NO_OP); } public OperationContext withSessionContext(final SessionContext sessionContext) { diff --git a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java index 75b7554d4d9..b92c2570f01 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java +++ b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java @@ -50,7 +50,7 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin @Override public AsyncClientExecutor getClientExecutor() { - return AsyncClientExecutor.unimplemented(); + return AsyncClientExecutor.NO_OP; } @Override diff --git a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java index 29175cfefaf..442b9af08ac 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java @@ -16,7 +16,6 @@ package com.mongodb.internal.thread; import com.mongodb.annotations.ThreadSafe; -import com.mongodb.assertions.Assertions; import com.mongodb.internal.async.SingleResultCallback; import com.mongodb.internal.connection.StreamFactoryFactory; @@ -45,12 +44,7 @@ */ @ThreadSafe public interface AsyncClientExecutor extends AutoCloseable { - /** - * The returned {@link AsyncClientExecutor} must not be used, as its methods {@linkplain Assertions#fail() fail}. - */ - static AsyncClientExecutor unimplemented() { - return UnimplementedAsyncClientExecutor.instance(); - } + AsyncClientExecutor NO_OP = UnimplementedAsyncClientExecutor.instance(); /** * @param executor The executor to use for executing tasks. diff --git a/driver-core/src/test/functional/com/mongodb/ClusterFixture.java b/driver-core/src/test/functional/com/mongodb/ClusterFixture.java index b2019a62e29..bcb2854a9b4 100644 --- a/driver-core/src/test/functional/com/mongodb/ClusterFixture.java +++ b/driver-core/src/test/functional/com/mongodb/ClusterFixture.java @@ -206,7 +206,7 @@ public static OperationContext createOperationContext() { } public static final InternalOperationContextFactory OPERATION_CONTEXT_FACTORY = - new InternalOperationContextFactory(TIMEOUT_SETTINGS, getServerApi(), AsyncClientExecutor.unimplemented()); + new InternalOperationContextFactory(TIMEOUT_SETTINGS, getServerApi(), AsyncClientExecutor.NO_OP); public static OperationContext createOperationContext(final TimeoutSettings timeoutSettings) { return new OperationContext( @@ -450,7 +450,7 @@ private static Cluster createCluster(final MongoCredential credential, final Str return new DefaultClusterFactory().createCluster(ClusterSettings.builder().hosts(asList(getPrimary())).build(), ServerSettings.builder().build(), ConnectionPoolSettings.builder().maxSize(1).build(), InternalConnectionPoolSettings.builder().build(), - TIMEOUT_SETTINGS.connectionOnly(), streamFactory, TIMEOUT_SETTINGS.connectionOnly(), streamFactory, AsyncClientExecutor.unimplemented(), credential, + TIMEOUT_SETTINGS.connectionOnly(), streamFactory, TIMEOUT_SETTINGS.connectionOnly(), streamFactory, AsyncClientExecutor.NO_OP, credential, LoggerSettings.builder().build(), null, null, null, Collections.emptyList(), getServerApi(), null); } @@ -462,7 +462,7 @@ private static Cluster createCluster(final ConnectionString connectionString, fi InternalConnectionPoolSettings.builder().build(), TimeoutSettings.create(mongoClientSettings).connectionOnly(), streamFactory, TimeoutSettings.createHeartbeatSettings(mongoClientSettings).connectionOnly(), new SocketStreamFactory(new DefaultInetAddressResolver(), SocketSettings.builder().readTimeout(5, SECONDS).build(), - getSslSettings(connectionString)), AsyncClientExecutor.unimplemented(), + getSslSettings(connectionString)), AsyncClientExecutor.NO_OP, connectionString.getCredential(), LoggerSettings.builder().build(), null, null, null, connectionString.getCompressorList(), getServerApi(), null); diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java index 1baf2d14053..5abcfbc1bb7 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java @@ -304,7 +304,7 @@ private OperationContext createOperationContext() { IgnorableRequestContext.INSTANCE, NoOpSessionContext.INSTANCE, new TimeoutContext(TIMEOUT_SETTINGS.withServerSelectionTimeoutMS(0)), - AsyncClientExecutor.unimplemented(), + AsyncClientExecutor.NO_OP, TracingManager.NO_OP, null, null, diff --git a/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy b/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy index 2a740caea71..60ec62b2faf 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolSpecification.groovy @@ -229,6 +229,6 @@ class ServerSessionPoolSpecification extends Specification { } static createServerSessionPool(Cluster cluster) { - new ServerSessionPool(cluster, AsyncClientExecutor.unimplemented(), TIMEOUT_SETTINGS, getServerApi()) + new ServerSessionPool(cluster, AsyncClientExecutor.NO_OP, TIMEOUT_SETTINGS, getServerApi()) } } diff --git a/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy b/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy index d7a36d5ea5a..f6a51e5607c 100644 --- a/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy +++ b/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy @@ -370,7 +370,7 @@ class MongoClientSpecification extends Specification { def mockStreamFactoryFactory() { Mock(StreamFactoryFactory) { - getClientExecutor() >> AsyncClientExecutor.unimplemented() + getClientExecutor() >> AsyncClientExecutor.NO_OP } } } diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java index ae3cf36f392..6dda49da685 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java @@ -183,7 +183,7 @@ private OperationContext createOperationContext(@Nullable final Timeout operatio throw new MongoOperationTimeoutException(TIMEOUT_ERROR_MESSAGE); }); } - return OperationContext.simpleOperationContext(timeoutSettings, null, AsyncClientExecutor.unimplemented()); + return OperationContext.simpleOperationContext(timeoutSettings, null, AsyncClientExecutor.NO_OP); } @NonNull diff --git a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java index e28410d1331..1ba8265273f 100644 --- a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java +++ b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java @@ -211,7 +211,7 @@ private MongoClientImpl createMongoClient() { .thenReturn(new ClientMetadata("test", mongoDriverInformation)); }); StreamFactoryFactory streamFactoryFactory = MongoMockito.mock(StreamFactoryFactory.class, mock -> { - when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.unimplemented()); + when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.NO_OP); }); return new MongoClientImpl(MongoClientSettings.builder().build(), mongoDriverInformation, cluster, streamFactoryFactory, OPERATION_EXECUTOR); diff --git a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java index 07e50f8dd77..720ae928380 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java @@ -79,7 +79,7 @@ void shouldCloseStreamFactoryFactory() { StreamFactoryFactory streamFactoryFactory = MongoMockito.mock( StreamFactoryFactory.class, mock -> { - when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.unimplemented()); + when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.NO_OP); try { doNothing().when(mock).close(); } catch (Exception e) { diff --git a/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy b/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy index 22a06f89d98..f8a825b96ab 100644 --- a/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy +++ b/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy @@ -229,7 +229,7 @@ class MongoClientSpecification extends Specification { def mockStreamFactoryFactory() { Mock(StreamFactoryFactory) { - getClientExecutor() >> AsyncClientExecutor.unimplemented() + getClientExecutor() >> AsyncClientExecutor.NO_OP } } } diff --git a/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy b/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy index db1937dcbd5..b6469cfb829 100644 --- a/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy +++ b/driver-sync/src/test/unit/com/mongodb/client/internal/MongoClusterSpecification.groovy @@ -261,6 +261,6 @@ class MongoClusterSpecification extends Specification { new MongoClusterImpl(null, cluster, settings.codecRegistry, null, null, originator, operationExecutor, settings.readConcern, settings.readPreference, settings.retryReads, settings.retryWrites, settings.enableOverloadRetargeting, null, serverSessionPool, TimeoutSettings.create(settings), settings.uuidRepresentation, - settings.writeConcern, AsyncClientExecutor.unimplemented(), TracingManager.NO_OP) + settings.writeConcern, AsyncClientExecutor.NO_OP, TracingManager.NO_OP) } } From 33f93bb86d51745d2e71926d25b42e91544aa2e0 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 28 Jul 2026 11:23:24 -0600 Subject: [PATCH 06/14] Remove dead code --- .../com/mongodb/ClusterFixture.java | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/driver-core/src/test/functional/com/mongodb/ClusterFixture.java b/driver-core/src/test/functional/com/mongodb/ClusterFixture.java index bcb2854a9b4..9dd1378d72b 100644 --- a/driver-core/src/test/functional/com/mongodb/ClusterFixture.java +++ b/driver-core/src/test/functional/com/mongodb/ClusterFixture.java @@ -75,9 +75,7 @@ import org.bson.BsonInt32; import org.bson.BsonString; import org.bson.BsonValue; -import org.bson.Document; import org.bson.codecs.BsonDocumentCodec; -import org.bson.codecs.DocumentCodec; import javax.net.ssl.SSLException; import java.time.Duration; @@ -144,7 +142,6 @@ public final class ClusterFixture { private static Cluster asyncCluster; private static final Map BINDING_MAP = new HashMap<>(); private static final Map SESSION_CONTEXT_MAP = new HashMap<>(); - private static final Map ASYNC_SESSION_CONTEXT_MAP = new HashMap<>(); private static final Map ASYNC_BINDING_MAP = new HashMap<>(); private static ServerVersion mongoCryptVersion; @@ -253,19 +250,6 @@ public static boolean hasEncryptionTestsEnabled() { .count() == requiredSystemProperties.size(); } - public static Document getServerStatus() { - return new CommandReadOperation<>("admin", new BsonDocument("serverStatus", new BsonInt32(1)), - new DocumentCodec()) - .execute(getBinding(), createOperationContext()); - } - - public static boolean supportsFsync() { - Document serverStatus = getServerStatus(); - Document storageEngine = (Document) serverStatus.get("storageEngine"); - - return storageEngine != null && !storageEngine.get("name").equals("inMemory"); - } - static class ShutdownHook extends Thread { @Override public void run() { @@ -414,7 +398,6 @@ public static AsyncReadWriteBinding getAsyncBinding( if (!ASYNC_BINDING_MAP.containsKey(readPreference)) { AsyncReadWriteBinding binding = new AsyncClusterBinding(cluster, readPreference); ASYNC_BINDING_MAP.put(readPreference, binding); - ASYNC_SESSION_CONTEXT_MAP.put(readPreference, new SimpleSessionContext()); } return ASYNC_BINDING_MAP.get(readPreference); } @@ -722,7 +705,7 @@ public static T executeAsync(final ReadOperation op, final AsyncReadBi return futureResultCallback.get(TIMEOUT, SECONDS); } - public static void loopCursor(final List> batchCursors, final Block block) throws Throwable { + public static void loopCursor(final List> batchCursors, final Block block) { List> futures = new ArrayList<>(); for (AsyncBatchCursor batchCursor : batchCursors) { FutureResultCallback futureResultCallback = new FutureResultCallback<>(); @@ -764,7 +747,7 @@ public static void loopCursor(final AsyncBatchCursor batchCursor, final B }); } - public static List collectCursorResults(final AsyncBatchCursor batchCursor) throws Throwable { + public static List collectCursorResults(final AsyncBatchCursor batchCursor) { List results = new ArrayList<>(); FutureResultCallback futureResultCallback = new FutureResultCallback<>(); loopCursor(batchCursor, t -> results.add(t), futureResultCallback); From 6479ad99e6c39a465eb6fb115dfcb459850d73b0 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Wed, 29 Jul 2026 04:34:29 -0600 Subject: [PATCH 07/14] Do the design changes as discussed --- .../com/mongodb/assertions/Assertions.java | 14 ++- ...nousSocketChannelStreamFactoryFactory.java | 3 +- .../internal/thread/CommonExecutor.java | 85 +++++-------- .../MongoScheduledThreadPoolExecutor.java | 15 +-- .../thread/MongoThreadPoolExecutor.java | 75 ++++++----- .../internal/thread/CommonExecutorTest.java | 52 ++++---- .../MongoScheduledThreadPoolExecutorTest.java | 36 +++--- .../thread/MongoThreadPoolExecutorTest.java | 116 ++++++++++-------- 8 files changed, 205 insertions(+), 191 deletions(-) diff --git a/driver-core/src/main/com/mongodb/assertions/Assertions.java b/driver-core/src/main/com/mongodb/assertions/Assertions.java index bf38638dc6d..e80e8189a90 100644 --- a/driver-core/src/main/com/mongodb/assertions/Assertions.java +++ b/driver-core/src/main/com/mongodb/assertions/Assertions.java @@ -205,7 +205,7 @@ public static boolean assertFalse(final boolean value) throws AssertionError { } /** - * @throws AssertionError Always + * @throws AssertionError Always. * @return Never completes normally. The return type is {@link AssertionError} to allow writing {@code throw fail()}. * This may be helpful in non-{@code void} methods. */ @@ -215,7 +215,7 @@ public static AssertionError fail() throws AssertionError { /** * @param msg The failure message. - * @throws AssertionError Always + * @throws AssertionError Always. * @return Never completes normally. The return type is {@link AssertionError} to allow writing {@code throw fail("failure message")}. * This may be helpful in non-{@code void} methods. */ @@ -223,6 +223,16 @@ public static AssertionError fail(final String msg) throws AssertionError { throw new AssertionError(assertNotNull(msg)); } + /** + * @param cause The {@linkplain AssertionError#getCause() cause}. + * @throws AssertionError Always. + * @return Never completes normally. The return type is {@link AssertionError} to allow writing {@code throw fail(cause)}. + * This may be helpful in non-{@code void} methods. + */ + public static AssertionError fail(final Throwable cause) throws AssertionError { + throw new AssertionError(null, cause); + } + /** * @param supplier the supplier to check * @return {@code supplier.get()} diff --git a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java index cc54a6db9ee..a79572cd8dd 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java @@ -19,7 +19,6 @@ import com.mongodb.connection.AsyncTransportSettings; import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; -import com.mongodb.internal.diagnostics.logging.Loggers; import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.internal.thread.DaemonThreadFactory; import com.mongodb.internal.thread.MongoThreadPoolExecutor; @@ -56,7 +55,7 @@ public AsynchronousSocketChannelStreamFactoryFactory(final InetAddressResolver i int availableProcessors = Runtime.getRuntime().availableProcessors(); ownedExecutorBackingClientExecutor = new MongoThreadPoolExecutor( availableProcessors, availableProcessors, Duration.ofMinutes(5), - new LinkedBlockingQueue<>(), new DaemonThreadFactory("ClientExecutor"), Loggers.getLogger("client")); + new LinkedBlockingQueue<>(), new DaemonThreadFactory("ClientExecutor")); ownedExecutorBackingClientExecutor.allowCoreThreadTimeOut(true); clientExecutor = AsyncClientExecutor.backedBy(ownedExecutorBackingClientExecutor); } diff --git a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java index 9461023b01b..3fb74e49cb2 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java @@ -22,27 +22,24 @@ import java.time.Duration; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; -import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import static com.mongodb.assertions.Assertions.fail; +import static java.lang.String.format; import static java.util.concurrent.TimeUnit.NANOSECONDS; -import static java.util.concurrent.TimeUnit.SECONDS; /** * A per-{@link ClassLoader} executor. *

* We do not always have access to {@link ScheduledExecutorService} in a {@code MongoClient}. - * For example, even if {@link AsyncTransportSettings#getExecutorService()} is present, it is merely an {@link ExecutorService}. + * For example, even if {@link AsyncTransportSettings#getExecutorService()} is present, it is merely an {@link ExecutorService}, + * and does not have to be a {@link ScheduledExecutorService}. * {@link CommonExecutor} is always accessible and may be used to schedule tasks when a more suitable alternative does not exist, * but must not be used to execute such scheduled tasks. *

- * Must not be used to execute blocking or application code. The only exception is {@link #uncaughtError(Error)}. - *

* This class is not part of the public API and may be removed or changed at any time. */ // VAKOTODO create a ticket and leave a TODO to use Cleaner when we are at Java SE 17 to shut down internal executors if the class is GCed. @@ -50,66 +47,52 @@ public final class CommonExecutor { private static final Logger LOGGER = Loggers.getLogger("client"); private static final CommonExecutor INSTANCE = new CommonExecutor(); - /** - * Exists solely to execute {@link ThreadGroup#uncaughtException(Thread, Throwable)}, - * which may execute application code and may be blocking. - * - * @see #uncaughtError(Error) - */ - private final ExecutorService uncaughtExceptionHandlerExecutor; - private final MongoScheduledThreadPoolExecutor scheduler; + private final MongoScheduledThreadPoolExecutor singleThreadScheduler; public static CommonExecutor commonExecutor() { return INSTANCE; } private CommonExecutor() { - // `uncaughtExceptionHandlerExecutor` is the only executor that must not be `MongoThreadPoolExecutor`/`MongoScheduledThreadPoolExecutor` - uncaughtExceptionHandlerExecutor = new ThreadPoolExecutor( - 0, 1, 5, SECONDS, new LinkedBlockingQueue<>(), new DaemonThreadFactory("CommonUncaughtExceptionHandler")); - scheduler = new MongoScheduledThreadPoolExecutor(1, new DaemonThreadFactory("CommonScheduler"), LOGGER); - } - - /** - * This method may be used in a situation when we cannot simply propagate an {@link Error} to the application. - * Handling an {@link Error} this way enables applications to decide how to deal with it via the - * {@linkplain Thread#getDefaultUncaughtExceptionHandler() default uncaught exception handler}. - *

- * An {@link Error} is more likely to cause an invariant violation than an {@link Exception}, - * because it is less likely to be taken into account in code. An {@link AssertionError} outright informs about an invariant violation. - * Furthermore, a {@link VirtualMachineError} not only may happen in peculiar situations, - * but also may be asynchronous. - * That is why it may be a good idea for an application to terminate on {@link Error}. - * We cannot make such a decision for an application, but we must do our best to give it an opportunity to react to an {@link Error}. - *

- * This method does not block. It also does not complete abruptly on the best-effort basis. - */ - void uncaughtError(final Error e) { - Thread t = Thread.currentThread(); - uncaughtExceptionHandlerExecutor.execute(() -> { - try { - t.getUncaughtExceptionHandler().uncaughtException(t, e); - } catch (Throwable ignored) { - // we are free to ignore the exception, and should ignore it - } - }); + singleThreadScheduler = new MongoScheduledThreadPoolExecutor(1, new DaemonThreadFactory("CommonScheduler")); } /** - * @param command The command to be scheduled. If it is {@link Executor#execute(Runnable) executed}, then the execution is guaranteed - * to be done via the {@code executor}. However, if the {@code executor} is shut down, then it does not execute the {@code command}, - * and there is nothing we can do about that, though {@link MongoScheduledThreadPoolExecutor#afterExecute(Runnable, Throwable)} - * logs the problem if {@link Executor#execute(Runnable)} completes abruptly. + * @param task The task to be scheduled. If it is {@link Executor#execute(Runnable) executed}, then the execution is guaranteed + * to be done via the {@code executor}. However, if the {@code executor} is shut down, then it does not execute the {@code task}, + * and there is nothing we can do about that. * @param delay A non-{@linkplain Duration#isNegative() negative} delay. - * @param executor The {@link Executor} to use for {@code command} {@linkplain Runnable#run() execution}, - * so that the {@code command} is not executed by a thread managed by {@link CommonExecutor}. + * @param executor The {@link Executor} to use for {@code task} {@linkplain Runnable#run() execution}, + * so that the {@code task} is not executed by a thread managed by {@link CommonExecutor}. * @return The {@link ScheduledFuture} representing only * the {@linkplain ScheduledExecutorService#schedule(Runnable, long, TimeUnit) scheduling part}, * and not the execution part done by the {@code executor}. */ - ScheduledFuture schedule(final Runnable command, final Duration delay, final Executor executor) { + ScheduledFuture schedule(final Runnable task, final Duration delay, final Executor executor) { try { - return scheduler.schedule(() -> executor.execute(command), delay.toNanos(), NANOSECONDS); + return singleThreadScheduler.schedule( + () -> { + // Depending on the `executor` implementation, which may be provided by an application, + // invoking `executor.execute` may result in executing arbitrary code, including `task`, in the single thread + // managed by `singleThreadScheduler`. This, in turn, may affect the behavior of other `MongoClient` instances + // that use `CommonExecutor`. More specifically, this may happen if: + // - `executor.execute` executes arbitrary code in the thread invoking the method; + // - `executor.execute` executes `task` in the thread invoking the `execute` method; + // - `executor` is a `ThreadPoolExecutor` with a bounded work queue that is full, + // and with `ThreadPoolExecutor.CallerRunsPolicy`; + // - `executor` is a `ThreadPoolExecutor` with a custom `RejectedExecutionHandler` that may result in + // executing `task` in the thread invoking the `execute` method. + // + // We consider the risk of the above small enough to make the current approach favourable to the alternative + // of having to run one more thread per `MongoClient`. + try { + executor.execute(task); + } catch (Exception e) { + LOGGER.error(format("The executor %s, which was likely provided by the application, either failed to execute" + + " the scheduled task %s, or executed it in the same thread that invoked `execute`", executor, task), e); + } + }, + delay.toNanos(), NANOSECONDS); } catch (RejectedExecutionException e) { throw fail(e.toString()); } diff --git a/driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java index 1ac2011f4ed..7d8e11b6714 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutor.java @@ -15,36 +15,27 @@ */ package com.mongodb.internal.thread; -import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.lang.Nullable; -import java.util.concurrent.Future; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadFactory; -import static com.mongodb.assertions.Assertions.assertNull; -import static com.mongodb.assertions.Assertions.assertTrue; -import static com.mongodb.internal.thread.MongoThreadPoolExecutor.handleTaskExceptions; +import static com.mongodb.internal.thread.MongoThreadPoolExecutor.propagateTaskFailureToUncaughtExceptionHandler; /** * See {@link MongoThreadPoolExecutor}. */ final class MongoScheduledThreadPoolExecutor extends ScheduledThreadPoolExecutor { - private final Logger logger; - MongoScheduledThreadPoolExecutor( final int corePoolSize, - final ThreadFactory threadFactory, - final Logger logger) { + final ThreadFactory threadFactory) { super(corePoolSize, threadFactory); setRemoveOnCancelPolicy(true); - this.logger = logger; } @Override protected void afterExecute(final Runnable r, @Nullable final Throwable t) { super.afterExecute(r, t); - assertTrue(r instanceof Future); - handleTaskExceptions(r, assertNull(t), logger); + propagateTaskFailureToUncaughtExceptionHandler(r, t); } } diff --git a/driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java index 05a46d00bbf..0fc245a894c 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/MongoThreadPoolExecutor.java @@ -15,9 +15,10 @@ */ package com.mongodb.internal.thread; -import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.lang.Nullable; +import java.io.PrintStream; +import java.lang.Thread.UncaughtExceptionHandler; import java.time.Duration; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CancellationException; @@ -27,65 +28,77 @@ import java.util.concurrent.ThreadPoolExecutor; import static com.mongodb.assertions.Assertions.assertNotNull; -import static com.mongodb.internal.thread.CommonExecutor.commonExecutor; +import static com.mongodb.assertions.Assertions.fail; import static java.util.concurrent.TimeUnit.NANOSECONDS; /** - * A {@link ThreadPoolExecutor} that always handles task exceptions, - * even if the caller ignores the {@link Future} that represents completion of the task: - *

    - *
  • - * {@link Error}s packed into {@link Future}s are handled via {@link CommonExecutor#uncaughtError(Error)}.
    • - *
  • - * All {@link Throwable}s are logged.
    • - *
+ * A {@link ThreadPoolExecutor} that ensures the task failure is propagated to the {@link UncaughtExceptionHandler}, if there is any, + * even if it is wrapped into a {@link Future} that represents completion of the task. This terminates the worker thread. + * Any non-{@link Error} gets turned into {@link AssertionError}, because all {@link Exception}s must be caught and handled by tasks. + *

+ * The driver code never observes task failures through {@link Future}s that represent completion of tasks, + * which is why this class is useful. + *

+ * Handling a task failure when it is an {@link Error} this way enables applications to decide how to deal with it + * via {@link UncaughtExceptionHandler}. An {@link Error} is more likely to cause an invariant violation than an {@link Exception}, + * because it is less likely to be taken into account in code. An {@link AssertionError} outright informs about an invariant violation. + * Furthermore, a {@link VirtualMachineError} not only may happen in a peculiar situation, + * but also may be asynchronous. + * That is why it may be a good idea for an application to terminate on {@link Error}. + * We cannot make such a decision for an application, but we must do our best to give it an opportunity to react to an {@link Error}. + *

+ * If there is no {@link UncaughtExceptionHandler}, then the failure is {@linkplain Throwable#printStackTrace(PrintStream) printed} + * to {@link System#err}, see {@link ThreadGroup#uncaughtException(Thread, Throwable)}. */ public final class MongoThreadPoolExecutor extends ThreadPoolExecutor { - private final Logger logger; - public MongoThreadPoolExecutor( final int corePoolSize, final int maximumPoolSize, final Duration keepAliveTime, final BlockingQueue workQueue, - final ThreadFactory threadFactory, - final Logger logger) { + final ThreadFactory threadFactory) { super(corePoolSize, maximumPoolSize, keepAliveTime.toNanos(), NANOSECONDS, workQueue, threadFactory); - this.logger = logger; } @Override protected void afterExecute(final Runnable r, @Nullable final Throwable t) { super.afterExecute(r, t); - handleTaskExceptions(r, t, logger); + propagateTaskFailureToUncaughtExceptionHandler(r, t); } - static void handleTaskExceptions(final Runnable r, @Nullable final Throwable t, final Logger logger) { - Throwable exception = getException(r, t); - if (exception instanceof Error && t == null) { - // the `Error` is held in `r`, and would have not been thrown to be handled by the uncaught exception handler - commonExecutor().uncaughtError((Error) exception); + static void propagateTaskFailureToUncaughtExceptionHandler(final Runnable maybeFuture, @Nullable final Throwable t) { + if (t != null) { + if (t instanceof Error) { + // nothing to do, as we know `t` is going to be thrown and caught by `UncaughtExceptionHandler`, if there is any + return; + } else { + throw fail(t); + } } - if (exception != null) { - logger.error("A task completed abruptly", exception); + Throwable tWrapped = unwrapThrowable(maybeFuture); + if (tWrapped != null) { + if (tWrapped instanceof Error) { + // we must throw `tWrapped` for it to be caught by `UncaughtExceptionHandler`, if there is any + throw (Error) tWrapped; + } else { + throw fail(tWrapped); + } } } @Nullable - private static Throwable getException(final Runnable r, @Nullable final Throwable t) { - if (t != null) { - return t; - } - if (r instanceof Future) { - Future runnableFuture = (Future) r; + private static Throwable unwrapThrowable(final Runnable maybeFuture) { + if (maybeFuture instanceof Future && ((Future) maybeFuture).isDone()) { + Future runnableFuture = (Future) maybeFuture; try { runnableFuture.get(); } catch (CancellationException e) { - return e; + // not a task failure + return null; } catch (ExecutionException e) { return assertNotNull(e.getCause()); } catch (InterruptedException e) { - // not else to do but to reinstate the interrupted status + // not a task failure Thread.currentThread().interrupt(); } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java index 5919543931a..e3690ddc077 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/CommonExecutorTest.java @@ -15,44 +15,38 @@ */ package com.mongodb.internal.thread; -import com.mongodb.lang.Nullable; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.lang.Thread.UncaughtExceptionHandler; +import java.time.Duration; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import static com.mongodb.internal.thread.CommonExecutor.commonExecutor; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertNotSame; class CommonExecutorTest { - private static final long TIMEOUT_MILLIS = SECONDS.toMillis(1); - - @Nullable - private UncaughtExceptionHandler originalUncaughtExceptionHandler; - - @BeforeEach - void beforeEach() { - originalUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler(); - } - - @AfterEach - void afterEach() { - if (originalUncaughtExceptionHandler != null) { - Thread.setDefaultUncaughtExceptionHandler(originalUncaughtExceptionHandler); - } - } - + private static final long TIMEOUT_MILLIS = 400; + /** + * This test verifies that even in the unlikely event that the single scheduling thread is terminated, + * it is replaced with another one to execute a previously scheduled task. + */ @Test - void uncaughtExceptionHandlerExecutesInDifferentThread() throws Exception { - CompletableFuture threadFuture = new CompletableFuture<>(); - Thread.setDefaultUncaughtExceptionHandler((t, e) -> { - threadFuture.complete(Thread.currentThread()); - }); - commonExecutor().uncaughtError(new AssertionError()); - assertNotSame(Thread.currentThread(), threadFuture.get(TIMEOUT_MILLIS, MILLISECONDS)); + void singleSchedulingThreadIsReplacedIfTerminated() throws Exception { + Executor sameThreadExecutor = Runnable::run; + CompletableFuture newSchedulingThread = new CompletableFuture<>(); + commonExecutor().schedule( + () -> newSchedulingThread.complete(Thread.currentThread()), + Duration.ofMillis(TIMEOUT_MILLIS / 2), + sameThreadExecutor); + CompletableFuture terminatedSchedulingThread = new CompletableFuture<>(); + commonExecutor().schedule( + () -> { + terminatedSchedulingThread.complete(Thread.currentThread()); + throw new Error("This error is thrown in the single scheduling thread, causing its termination"); + }, + Duration.ZERO, + sameThreadExecutor); + assertNotSame(terminatedSchedulingThread.get(TIMEOUT_MILLIS, MILLISECONDS), newSchedulingThread.get(TIMEOUT_MILLIS, MILLISECONDS)); } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java index 6308b77b701..14d2a7ce2ef 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoScheduledThreadPoolExecutorTest.java @@ -16,7 +16,7 @@ package com.mongodb.internal.thread; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.params.provider.CsvSource; import java.util.concurrent.Callable; @@ -24,14 +24,20 @@ class MongoScheduledThreadPoolExecutorTest extends MongoThreadPoolExecutorTest { @ParameterizedTest - @ValueSource(booleans = {true, false}) + @CsvSource({ + "false, false", + "false, true", + "true, false", + "true, true"}) @Override - void delegateErrorToDefaultUncaughtExceptionHandlerAndLog(final boolean taskCompletesAbruptlyWithError) throws Exception { - MongoScheduledThreadPoolExecutor executor = new MongoScheduledThreadPoolExecutor(1, new DaemonThreadFactory("test"), getLogger()); + void delegateErrorToDefaultUncaughtExceptionHandlerOrLog( + final boolean taskCompletesAbruptlyWithError, + final boolean setDefaultUncaughtExceptionHandler) throws Exception { + MongoScheduledThreadPoolExecutor executor = new MongoScheduledThreadPoolExecutor(1, new DaemonThreadFactory("test")); try { - Error error = new AssertionError(); - RuntimeException exception = new RuntimeException(); - Throwable expectedException = taskCompletesAbruptlyWithError ? error : exception; + Error error = new Error("expected error"); + RuntimeException exception = new RuntimeException("expected exception"); + Throwable expectedThrowable = taskCompletesAbruptlyWithError ? error : exception; Runnable runnable = () -> { if (taskCompletesAbruptlyWithError) { throw error; @@ -43,14 +49,14 @@ void delegateErrorToDefaultUncaughtExceptionHandlerAndLog(final boolean taskComp runnable.run(); return null; }; - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.execute(runnable)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.submit(runnable)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.submit(runnable, null)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.submit(callable)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.schedule(runnable, 0, MILLISECONDS)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.schedule(callable, 0, MILLISECONDS)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.scheduleAtFixedRate(runnable, 0, 1, MILLISECONDS)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedException, false, () -> executor.scheduleWithFixedDelay(runnable, 0, 1, MILLISECONDS)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.execute(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.submit(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.submit(runnable, null)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.submit(callable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.schedule(runnable, 0, MILLISECONDS)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.schedule(callable, 0, MILLISECONDS)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.scheduleAtFixedRate(runnable, 0, 1, MILLISECONDS)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.scheduleWithFixedDelay(runnable, 0, 1, MILLISECONDS)); } finally { executor.shutdownNow(); } diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java index 5615c34887d..f17c87a4bce 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/MongoThreadPoolExecutorTest.java @@ -15,61 +15,77 @@ */ package com.mongodb.internal.thread; -import com.mongodb.internal.diagnostics.logging.Logger; -import com.mongodb.internal.diagnostics.logging.Loggers; import com.mongodb.lang.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.params.provider.CsvSource; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; import java.lang.Thread.UncaughtExceptionHandler; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ScheduledThreadPoolExecutor; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.matches; -import static org.mockito.Mockito.clearInvocations; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; +import static org.junit.jupiter.api.Assertions.assertTrue; class MongoThreadPoolExecutorTest { private static final long TIMEOUT_MILLIS = 100; - private Logger logger; - + @Nullable + private PrintStream originalStderr; + private ByteArrayOutputStream stderrTap; @Nullable private UncaughtExceptionHandler originalUncaughtExceptionHandler; @BeforeEach - void beforeEach() { - logger = spy(Loggers.getLogger("test")); + void beforeEach() throws UnsupportedEncodingException { originalUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler(); + originalStderr = System.err; + stderrTap = new ByteArrayOutputStream(); + System.setErr(new PrintStream(stderrTap, true, StandardCharsets.UTF_8.name())); } @AfterEach - void afterEach() { - if (originalUncaughtExceptionHandler != null) { + void afterEach() throws IOException { + try { Thread.setDefaultUncaughtExceptionHandler(originalUncaughtExceptionHandler); + } finally { + try { + if (originalStderr != null) { + System.setErr(originalStderr); + } + } finally { + if (stderrTap != null) { + stderrTap.close(); + } + } } } @ParameterizedTest - @ValueSource(booleans = {true, false}) - void delegateErrorToDefaultUncaughtExceptionHandlerAndLog(final boolean taskCompletesAbruptlyWithError) throws Exception { + @CsvSource({ + "false, false", + "false, true", + "true, false", + "true, true" + }) + void delegateErrorToDefaultUncaughtExceptionHandlerOrLog( + final boolean taskCompletesAbruptlyWithError, + final boolean setDefaultUncaughtExceptionHandler) throws Exception { MongoThreadPoolExecutor executor = new MongoThreadPoolExecutor( - 1, 1, Duration.ofMillis(TIMEOUT_MILLIS), new LinkedBlockingQueue<>(), new DaemonThreadFactory("test"), logger); + 1, 1, Duration.ofMillis(TIMEOUT_MILLIS), new LinkedBlockingQueue<>(), new DaemonThreadFactory("test")); try { - Error error = new AssertionError(); - RuntimeException exception = new RuntimeException(); + Error error = new Error("expected error"); + RuntimeException exception = new RuntimeException("expected exception"); Throwable expectedThrowable = taskCompletesAbruptlyWithError ? error : exception; Runnable runnable = () -> { if (taskCompletesAbruptlyWithError) { @@ -82,44 +98,46 @@ void delegateErrorToDefaultUncaughtExceptionHandlerAndLog(final boolean taskComp runnable.run(); return null; }; - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, true, () -> executor.execute(runnable)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, false, () -> executor.submit(runnable)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, false, () -> executor.submit(runnable, null)); - assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog(expectedThrowable, false, () -> executor.submit(callable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.execute(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.submit(runnable)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.submit(runnable, null)); + assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog(expectedThrowable, setDefaultUncaughtExceptionHandler, () -> executor.submit(callable)); } finally { executor.shutdownNow(); } } - /** - * @param delegatesExceptionToUncaughtExceptionHandler The {@link ExecutorService#execute(Runnable)} method usually results in - * the task exception to be thrown from the worker thread {@link Thread#run()} method, - * though {@link ScheduledThreadPoolExecutor#execute(Runnable)} is an example where that is not the case. - * This is different from the behavior of the {@link ExecutorService#submit(Runnable)} method, and similar methods that return - * a {@link Future} holding the task exception. - * This parameter reflects the expected behavior, depending on the method used by {@code submitThrowingTask}. - */ - void assertDelegateErrorToDefaultUncaughtExceptionHandlerAndLog( + void assertDelegateErrorToDefaultUncaughtExceptionHandlerOrLog( final Throwable expectedThrowable, - final boolean delegatesExceptionToUncaughtExceptionHandler, + final boolean setDefaultUncaughtExceptionHandler, final Runnable submitThrowingTask) throws Exception { CompletableFuture uncaughtExceptionFuture = new CompletableFuture<>(); - Thread.setDefaultUncaughtExceptionHandler((t, e) -> { - uncaughtExceptionFuture.complete(e); - }); - clearInvocations(logger); + if (setDefaultUncaughtExceptionHandler) { + Thread.setDefaultUncaughtExceptionHandler((t, e) -> { + uncaughtExceptionFuture.complete(e); + }); + } else { + // we remove the original `UncaughtExceptionHandler` to guarantee that uncaught exceptions are printed to `System.err` + Thread.setDefaultUncaughtExceptionHandler(null); + } + stderrTap.reset(); submitThrowingTask.run(); Thread.sleep(TIMEOUT_MILLIS); - if (expectedThrowable instanceof Error || delegatesExceptionToUncaughtExceptionHandler) { + if (setDefaultUncaughtExceptionHandler) { Throwable actualUncaughtException = uncaughtExceptionFuture.get(TIMEOUT_MILLIS, MILLISECONDS); - assertSame(expectedThrowable, actualUncaughtException); + if (expectedThrowable instanceof Error) { + assertSame(expectedThrowable, actualUncaughtException); + } else { + assertInstanceOf(Error.class, actualUncaughtException); + assertSame(expectedThrowable, actualUncaughtException.getCause()); + } } else { - assertFalse(uncaughtExceptionFuture.isDone()); + String actualLoggedMessage = stderrTap.toString(StandardCharsets.UTF_8.name()); + assertTrue(actualLoggedMessage.contains(expectedThrowable.getClass().getName()) && actualLoggedMessage.contains(expectedThrowable.getMessage()), + () -> { + return String.format("actualLoggedMessage=%s does not contain information about expectedThrowable=%s", + actualLoggedMessage, expectedThrowable); + }); } - verify(logger).error(matches("A task completed abruptly"), any()); - } - - Logger getLogger() { - return logger; } } From 13d9cc3ec413abb98af3729955658c748db2a646 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Thu, 6 Aug 2026 17:56:34 -0600 Subject: [PATCH 08/14] Simplify `RetryingSyncSupplier` and improve the assertion in `CommonExecutor` --- .../internal/async/function/RetryingSyncSupplier.java | 9 ++------- .../main/com/mongodb/internal/thread/CommonExecutor.java | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java index 124e6aa013e..75c616e88da 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java @@ -16,7 +16,6 @@ package com.mongodb.internal.async.function; import com.mongodb.annotations.NotThreadSafe; -import com.mongodb.internal.async.MutableValue; import com.mongodb.internal.async.function.RetryPolicy.Decision.RetryAttemptInfo; import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; @@ -53,12 +52,9 @@ public RetryingSyncSupplier(final RetryControl control, final Supplier syn @Override @Nullable public R get() { - MutableValue> asyncFunctionSuccessfulResult = new MutableValue<>(); - while (asyncFunctionSuccessfulResult.getNullable() == null) { + while (true) { try { - R attemptSuccessfulResult = syncFunction.get(); - // `attemptSuccessfulResult` may be `null`, so we have to wrap it in `MutableValue` for the while check to notice it - asyncFunctionSuccessfulResult.set(new MutableValue<>(attemptSuccessfulResult)); + return syncFunction.get(); } catch (Error attemptFailedResult) { throw attemptFailedResult; } catch (Throwable attemptFailedResult) { @@ -66,7 +62,6 @@ public R get() { sleep(retryAttemptInfo.getBackoff()); } } - return asyncFunctionSuccessfulResult.get().getNullable(); } private static void sleep(final Duration duration) { diff --git a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java index 3fb74e49cb2..064d22146ca 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java @@ -94,7 +94,7 @@ ScheduledFuture schedule(final Runnable task, final Duration delay, final Exe }, delay.toNanos(), NANOSECONDS); } catch (RejectedExecutionException e) { - throw fail(e.toString()); + throw fail(e); } } } From 171808cc2ae16b798d8a2cc53ae992efadb258d7 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 10 Aug 2026 18:35:54 -0600 Subject: [PATCH 09/14] Make `MongoClientImpl` constructors the same for sync and reactive --- .../src/main/com/mongodb/MongoClient.java | 2 +- .../reactivestreams/client/MongoClients.java | 2 +- .../client/internal/MongoClientImpl.java | 20 ++++++++++--------- .../client/internal/MongoClientImplTest.java | 3 +-- .../main/com/mongodb/client/MongoClients.java | 2 +- .../client/internal/MongoClientImpl.java | 20 ++++++++++--------- .../com/mongodb/client/MongoClientTest.java | 2 +- 7 files changed, 27 insertions(+), 24 deletions(-) diff --git a/driver-legacy/src/main/com/mongodb/MongoClient.java b/driver-legacy/src/main/com/mongodb/MongoClient.java index de1d7e641bc..426b0e6f152 100644 --- a/driver-legacy/src/main/com/mongodb/MongoClient.java +++ b/driver-legacy/src/main/com/mongodb/MongoClient.java @@ -262,7 +262,7 @@ private MongoClient(final MongoClientSettings settings, wrappedMongoDriverInformation, syncStreamFactoryFactory); - delegate = new MongoClientImpl(cluster, settings, wrappedMongoDriverInformation, syncStreamFactoryFactory); + delegate = new MongoClientImpl(cluster, wrappedMongoDriverInformation, settings, syncStreamFactoryFactory); this.options = options != null ? options : MongoClientOptions.builder(settings).build(); cursorCleaningService = this.options.isCursorFinalizerEnabled() ? createCursorCleaningService() : null; this.closed = new AtomicBoolean(); diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java index da722d16ba0..922ca40b121 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java @@ -120,7 +120,7 @@ public static MongoClient create(final MongoClientSettings settings, @Nullable f StreamFactory heartbeatStreamFactory = getStreamFactory(streamFactoryFactory, settings, true); MongoDriverInformation wrappedMongoDriverInformation = wrapMongoDriverInformation(mongoDriverInformation); Cluster cluster = createCluster(settings, wrappedMongoDriverInformation, streamFactory, heartbeatStreamFactory, streamFactoryFactory.getClientExecutor()); - return new MongoClientImpl(settings, wrappedMongoDriverInformation, cluster, streamFactoryFactory); + return new MongoClientImpl(cluster, wrappedMongoDriverInformation, settings, streamFactoryFactory); } /** diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java index dbcc27f5022..9b803e222f2 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java @@ -78,19 +78,21 @@ public final class MongoClientImpl implements MongoClient { private final MongoClusterImpl delegate; private final AtomicBoolean closed; - public MongoClientImpl(final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, final Cluster cluster, + public MongoClientImpl( + final Cluster cluster, + final MongoDriverInformation mongoDriverInformation, + final MongoClientSettings settings, final StreamFactoryFactory streamFactoryFactory) { - this(settings, mongoDriverInformation, cluster, null, streamFactoryFactory); + this(cluster, mongoDriverInformation, settings, streamFactoryFactory, null); } @VisibleForTesting(otherwise = PRIVATE) - MongoClientImpl(final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, final Cluster cluster, - final StreamFactoryFactory streamFactoryFactory, @Nullable final OperationExecutor executor) { - this(settings, mongoDriverInformation, cluster, executor, streamFactoryFactory); - } - - private MongoClientImpl(final MongoClientSettings settings, final MongoDriverInformation mongoDriverInformation, final Cluster cluster, - @Nullable final OperationExecutor executor, final StreamFactoryFactory streamFactoryFactory) { + MongoClientImpl( + final Cluster cluster, + final MongoDriverInformation mongoDriverInformation, + final MongoClientSettings settings, + final StreamFactoryFactory streamFactoryFactory, + @Nullable final OperationExecutor executor) { notNull("settings", settings); notNull("cluster", cluster); diff --git a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java index 1ba8265273f..490bdce7ad4 100644 --- a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java +++ b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java @@ -213,7 +213,6 @@ private MongoClientImpl createMongoClient() { StreamFactoryFactory streamFactoryFactory = MongoMockito.mock(StreamFactoryFactory.class, mock -> { when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.NO_OP); }); - return new MongoClientImpl(MongoClientSettings.builder().build(), - mongoDriverInformation, cluster, streamFactoryFactory, OPERATION_EXECUTOR); + return new MongoClientImpl(cluster, mongoDriverInformation, MongoClientSettings.builder().build(), streamFactoryFactory, OPERATION_EXECUTOR); } } diff --git a/driver-sync/src/main/com/mongodb/client/MongoClients.java b/driver-sync/src/main/com/mongodb/client/MongoClients.java index e0e59ba5f78..9ce2e87ea70 100644 --- a/driver-sync/src/main/com/mongodb/client/MongoClients.java +++ b/driver-sync/src/main/com/mongodb/client/MongoClients.java @@ -126,7 +126,7 @@ public static MongoClient create(final MongoClientSettings settings, @Nullable f driverInfo, syncStreamFactoryFactory); - return new MongoClientImpl(cluster, settings, driverInfo, syncStreamFactoryFactory); + return new MongoClientImpl(cluster, driverInfo, settings, syncStreamFactoryFactory); } private MongoClients() { diff --git a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java index 2cd7ffffa5d..73e4ead5388 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java @@ -74,19 +74,21 @@ public final class MongoClientImpl implements MongoClient { private final AtomicBoolean closed; private final StreamFactoryFactory streamFactoryFactory; - public MongoClientImpl(final Cluster cluster, - final MongoClientSettings settings, - final MongoDriverInformation mongoDriverInformation, - final StreamFactoryFactory streamFactoryFactory) { + public MongoClientImpl( + final Cluster cluster, + final MongoDriverInformation mongoDriverInformation, + final MongoClientSettings settings, + final StreamFactoryFactory streamFactoryFactory) { this(cluster, mongoDriverInformation, settings, streamFactoryFactory, null); } @VisibleForTesting(otherwise = PRIVATE) - public MongoClientImpl(final Cluster cluster, - final MongoDriverInformation mongoDriverInformation, - final MongoClientSettings settings, - final StreamFactoryFactory streamFactoryFactory, - @Nullable final OperationExecutor operationExecutor) { + public MongoClientImpl( + final Cluster cluster, + final MongoDriverInformation mongoDriverInformation, + final MongoClientSettings settings, + final StreamFactoryFactory streamFactoryFactory, + @Nullable final OperationExecutor operationExecutor) { this.streamFactoryFactory = streamFactoryFactory; this.settings = notNull("settings", settings); diff --git a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java index 720ae928380..3565ad6c7d5 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java @@ -89,8 +89,8 @@ void shouldCloseStreamFactoryFactory() { MongoClientImpl mongoClient = new MongoClientImpl( cluster, - MongoClientSettings.builder().build(), mongoDriverInformation, + MongoClientSettings.builder().build(), streamFactoryFactory); //when From 7e8108c822bb7572a138482cc1d6df736b8cfa23 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Thu, 13 Aug 2026 07:06:10 -0600 Subject: [PATCH 10/14] Make `StreamFactoryFactory` return its I/O executor instead of `AsyncClientExecutor` --- ...nousSocketChannelStreamFactoryFactory.java | 30 +++----- .../connection/StreamFactoryFactory.java | 13 +++- .../connection/StreamFactoryHelper.java | 8 ++- .../TlsChannelStreamFactoryFactory.java | 20 ++---- .../netty/NettyStreamFactoryFactory.java | 24 +++---- .../internal/thread/AsyncClientExecutor.java | 2 +- .../internal/thread/CommonExecutor.java | 4 +- .../internal/mockito/MongoMockito.java | 11 +-- .../src/main/com/mongodb/MongoClient.java | 8 ++- .../mongodb/MongoClientSpecification.groovy | 9 ++- .../reactivestreams/client/MongoClients.java | 5 +- .../client/internal/MongoClientImpl.java | 22 +++--- .../client/internal/MongoClientImplTest.java | 18 +++-- .../main/com/mongodb/client/MongoClients.java | 8 ++- .../com/mongodb/client/internal/Clusters.java | 6 +- .../client/internal/MongoClientImpl.java | 21 +++--- .../com/mongodb/client/MongoClientTest.java | 68 +++++++++++-------- .../client/MongoClientSpecification.groovy | 22 ++++-- 18 files changed, 166 insertions(+), 133 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java index a79572cd8dd..bde53275043 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java @@ -19,7 +19,6 @@ import com.mongodb.connection.AsyncTransportSettings; import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; -import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.internal.thread.DaemonThreadFactory; import com.mongodb.internal.thread.MongoThreadPoolExecutor; import com.mongodb.lang.Nullable; @@ -41,7 +40,6 @@ public final class AsynchronousSocketChannelStreamFactoryFactory implements Stre @Nullable private final AsynchronousChannelGroup group; private final MongoThreadPoolExecutor ownedExecutorBackingClientExecutor; - private final AsyncClientExecutor clientExecutor; public AsynchronousSocketChannelStreamFactoryFactory(final InetAddressResolver inetAddressResolver) { this(inetAddressResolver, null); @@ -57,7 +55,6 @@ public AsynchronousSocketChannelStreamFactoryFactory(final InetAddressResolver i availableProcessors, availableProcessors, Duration.ofMinutes(5), new LinkedBlockingQueue<>(), new DaemonThreadFactory("ClientExecutor")); ownedExecutorBackingClientExecutor.allowCoreThreadTimeOut(true); - clientExecutor = AsyncClientExecutor.backedBy(ownedExecutorBackingClientExecutor); } @Override @@ -67,31 +64,24 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin } /** - * VAKOTODO create ticket, leave a TODO - * To make things right, this should be - * the {@linkplain AsyncClientExecutor} {@linkplain AsyncClientExecutor#backedBy(Executor) backed by} same {@link ExecutorService} that - * the {@link AsynchronousChannelGroup} in {@link AsynchronousSocketChannelStreamFactory} uses - * (see {@link #create(SocketSettings, SslSettings)}). - * That {@link ExecutorService} may be provided by an application via {@link AsyncTransportSettings#getExecutorService()}. - * But currently it is a separate {@link MongoThreadPoolExecutor}. + * @return VAKOTODO create ticket, leave a TODO + * A dedicated {@link MongoThreadPoolExecutor}, which is suboptimal. To make things right, this should be + * The {@link ExecutorService} used by the {@link StreamFactory} created via {@link #create(SocketSettings, SslSettings)}. + * It may be provided by an application via {@link AsyncTransportSettings#getExecutorService()}. */ @Override - public AsyncClientExecutor getClientExecutor() { - return clientExecutor; + public Executor getExecutor() { + return ownedExecutorBackingClientExecutor; } @Override public void close() { try { - clientExecutor.close(); - } finally { - try { - if (group != null) { - group.shutdown(); - } - } finally { - ownedExecutorBackingClientExecutor.shutdown(); + if (group != null) { + group.shutdown(); } + } finally { + ownedExecutorBackingClientExecutor.shutdown(); } } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java index 8f3408d7bc5..589a20bb8e1 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryFactory.java @@ -18,7 +18,10 @@ import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; -import com.mongodb.internal.thread.AsyncClientExecutor; +import com.mongodb.connection.TransportSettings; + +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; /** * A factory of {@code StreamFactory} instances. @@ -34,7 +37,13 @@ public interface StreamFactoryFactory extends AutoCloseable { */ StreamFactory create(SocketSettings socketSettings, SslSettings sslSettings); - AsyncClientExecutor getClientExecutor(); + /** + * The {@link Executor} that the {@link StreamFactoryFactory} uses for I/O. + * It is usually {@link ExecutorService#shutdown() shut down} by {@link #close()}, + * but the behavior may be different if an application provides an executor via {@link TransportSettings}. + * See the documentation of the corresponding API for information about the lifecycle of an application-provided executor. + */ + Executor getExecutor(); @Override void close(); diff --git a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java index b92c2570f01..b781bf9018b 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java +++ b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java @@ -24,14 +24,16 @@ import com.mongodb.connection.SslSettings; import com.mongodb.connection.TransportSettings; import com.mongodb.internal.connection.netty.NettyStreamFactoryFactory; -import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; import java.io.IOException; import java.nio.channels.AsynchronousChannelGroup; +import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; +import static com.mongodb.assertions.Assertions.fail; + /** *

This class is not part of the public API and may be removed or changed at any time

*/ @@ -49,8 +51,8 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin } @Override - public AsyncClientExecutor getClientExecutor() { - return AsyncClientExecutor.NO_OP; + public Executor getExecutor() { + throw fail(); } @Override diff --git a/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java index b3d633221f3..4296cb01392 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/TlsChannelStreamFactoryFactory.java @@ -30,7 +30,6 @@ import com.mongodb.internal.connection.tlschannel.async.AsynchronousTlsChannelGroup; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; -import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; @@ -71,7 +70,6 @@ public class TlsChannelStreamFactoryFactory implements StreamFactoryFactory { private final SelectorMonitor selectorMonitor; private final AsynchronousTlsChannelGroup group; - private final AsyncClientExecutor clientExecutor; private final PowerOfTwoBufferPool bufferPool = PowerOfTwoBufferPool.DEFAULT; private final InetAddressResolver inetAddressResolver; @@ -82,7 +80,6 @@ public class TlsChannelStreamFactoryFactory implements StreamFactoryFactory { @Nullable final ExecutorService executorService) { this.inetAddressResolver = inetAddressResolver; this.group = new AsynchronousTlsChannelGroup(executorService); - clientExecutor = AsyncClientExecutor.backedBy(group); selectorMonitor = new SelectorMonitor(); selectorMonitor.start(); } @@ -99,25 +96,20 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin } /** - * The {@linkplain AsyncClientExecutor} {@linkplain AsyncClientExecutor#backedBy(Executor) backed by} the same {@link ExecutorService} - * used by {@link #create(SocketSettings, SslSettings)} for a {@link StreamFactory}. - * That {@link ExecutorService} may be provided by an application via {@link AsyncTransportSettings#getExecutorService()}. + * @return The {@link Executor} used by the {@link StreamFactory} created via {@link #create(SocketSettings, SslSettings)}. + * It may be backed by the {@link ExecutorService} provided by an application via {@link AsyncTransportSettings#getExecutorService()}. */ @Override - public AsyncClientExecutor getClientExecutor() { - return clientExecutor; + public Executor getExecutor() { + return group; } @Override public void close() { try { - clientExecutor.close(); + selectorMonitor.close(); } finally { - try { - selectorMonitor.close(); - } finally { - group.shutdown(); - } + group.shutdown(); } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java index 6a31ef3a9a0..a939092f952 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyStreamFactoryFactory.java @@ -22,7 +22,6 @@ import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.connection.StreamFactory; import com.mongodb.internal.connection.StreamFactoryFactory; -import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; import io.netty.buffer.ByteBufAllocator; @@ -50,7 +49,6 @@ public final class NettyStreamFactoryFactory implements StreamFactoryFactory { private final EventLoopGroup eventLoopGroup; private final boolean ownsEventLoopGroup; - private final AsyncClientExecutor clientExecutor; private final Class socketChannelClass; private final ByteBufAllocator allocator; @Nullable @@ -207,25 +205,20 @@ public StreamFactory create(final SocketSettings socketSettings, final SslSettin } /** - * The {@linkplain AsyncClientExecutor} {@linkplain AsyncClientExecutor#backedBy(Executor) backed by} the same {@link EventLoopGroup} - * used by {@link #create(SocketSettings, SslSettings)} for a {@link StreamFactory}. - * That {@link EventLoopGroup} may be provided by an application via {@link NettyTransportSettings#getEventLoopGroup()}. + * @return The {@link EventLoopGroup} used by the {@link StreamFactory} created via {@link #create(SocketSettings, SslSettings)}. + * It may be provided by an application via {@link NettyTransportSettings#getEventLoopGroup()}. */ @Override - public AsyncClientExecutor getClientExecutor() { - return clientExecutor; + public Executor getExecutor() { + return eventLoopGroup; } @Override public void close() { - try { - clientExecutor.close(); - } finally { - if (ownsEventLoopGroup) { - // ignore the returned Future. This is in line with MongoClient behavior to not block waiting for connections to be returned - // to the pool - eventLoopGroup.shutdownGracefully(); - } + if (ownsEventLoopGroup) { + // ignore the returned Future. This is in line with MongoClient behavior to not block waiting for connections to be returned + // to the pool + eventLoopGroup.shutdownGracefully(); } } @@ -253,7 +246,6 @@ private NettyStreamFactoryFactory(final Builder builder) { socketChannelClass = builder.socketChannelClass == null ? NioSocketChannel.class : builder.socketChannelClass; eventLoopGroup = builder.eventLoopGroup == null ? new NioEventLoopGroup() : builder.eventLoopGroup; ownsEventLoopGroup = builder.eventLoopGroup == null; - clientExecutor = AsyncClientExecutor.backedBy(eventLoopGroup); sslContext = builder.sslContext; inetAddressResolver = builder.inetAddressResolver; } diff --git a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java index 442b9af08ac..319074e44f5 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java @@ -39,7 +39,6 @@ *

* This class is not part of the public API and may be removed or changed at any time. * - * @see StreamFactoryFactory#getClientExecutor() * @see CommonExecutor */ @ThreadSafe @@ -50,6 +49,7 @@ public interface AsyncClientExecutor extends AutoCloseable { * @param executor The executor to use for executing tasks. * If it is a {@link ScheduledExecutorService}, then it is also used for scheduling, * otherwise {@link CommonExecutor} is used for scheduling. + * @see StreamFactoryFactory#getExecutor() */ static AsyncClientExecutor backedBy(final Executor executor) { return new DefaultAsyncClientExecutor(executor); diff --git a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java index 064d22146ca..0386073c7d8 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java @@ -41,8 +41,10 @@ * but must not be used to execute such scheduled tasks. *

* This class is not part of the public API and may be removed or changed at any time. + * + * @see AsyncClientExecutor */ -// VAKOTODO create a ticket and leave a TODO to use Cleaner when we are at Java SE 17 to shut down internal executors if the class is GCed. +// VAKOTODO decide what to do with https://jira.mongodb.org/browse/JAVA-6279. public final class CommonExecutor { private static final Logger LOGGER = Loggers.getLogger("client"); private static final CommonExecutor INSTANCE = new CommonExecutor(); diff --git a/driver-core/src/test/unit/com/mongodb/internal/mockito/MongoMockito.java b/driver-core/src/test/unit/com/mongodb/internal/mockito/MongoMockito.java index 7b6c08a2efb..739aaf996a0 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/mockito/MongoMockito.java +++ b/driver-core/src/test/unit/com/mongodb/internal/mockito/MongoMockito.java @@ -23,7 +23,7 @@ import java.util.function.Consumer; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.withSettings; /** @@ -56,18 +56,19 @@ public static T mock(final Class classToMock) { * Moreover, a mock object created with {@link ThrowsException} as its default answer cannot be stubbed: * stubbing requires calling methods of the mock object, but they all complete abruptly * (see {@link InsufficientStubbingDetectorDemoTest#stubbingWithThrowsException()}). - * Therefore, {@link ThrowsException} is not suitable for detecting insufficient stubbing.

+ * Therefore, {@link ThrowsException} is not suitable for detecting insufficient stubbing. *

* This method overcomes both of the aforementioned limitations by using {@link InsufficientStubbingDetector} as the default answer * (see {@link InsufficientStubbingDetectorDemoTest#mockObjectWithInsufficientStubbingDetector()}, * {@link InsufficientStubbingDetectorDemoTest#stubbingWithInsufficientStubbingDetector()}). - * Note also that for convenience, {@link InsufficientStubbingDetector} stubs the {@link Object#toString()} method by using - * {@link OngoingStubbing#thenCallRealMethod()}, unless this stubbing is overwritten by the {@code tuner}.

+ * Note also that for convenience, {@link InsufficientStubbingDetector} {@linkplain Mockito#lenient() leniently} + * stubs the {@link Object#toString()} method by using + * {@link OngoingStubbing#thenCallRealMethod()}, unless this stubbing is overwritten by the {@code tuner}. */ public static T mock(final Class classToMock, @Nullable final Consumer tuner) { final InsufficientStubbingDetector insufficientStubbingDetector = new InsufficientStubbingDetector(); final T mock = Mockito.mock(classToMock, withSettings().defaultAnswer(insufficientStubbingDetector)); - when(mock.toString()).thenCallRealMethod(); + lenient().when(mock.toString()).thenCallRealMethod(); if (tuner != null) { tuner.accept(mock); } diff --git a/driver-legacy/src/main/com/mongodb/MongoClient.java b/driver-legacy/src/main/com/mongodb/MongoClient.java index 426b0e6f152..fbbbab0f817 100644 --- a/driver-legacy/src/main/com/mongodb/MongoClient.java +++ b/driver-legacy/src/main/com/mongodb/MongoClient.java @@ -43,6 +43,7 @@ import com.mongodb.internal.diagnostics.logging.Loggers; import com.mongodb.internal.observability.micrometer.TracingManager; import com.mongodb.internal.session.ServerSessionPool; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.internal.thread.DaemonThreadFactory; import com.mongodb.internal.validator.NoOpFieldNameValidator; import com.mongodb.lang.Nullable; @@ -256,13 +257,14 @@ private MongoClient(final MongoClientSettings settings, StreamFactoryFactory syncStreamFactoryFactory = getSyncStreamFactoryFactory( settings.getTransportSettings(), getInetAddressResolver(settings)); - + AsyncClientExecutor clientExecutor = AsyncClientExecutor.NO_OP; Cluster cluster = Clusters.createCluster( settings, wrappedMongoDriverInformation, - syncStreamFactoryFactory); + syncStreamFactoryFactory, + clientExecutor); - delegate = new MongoClientImpl(cluster, wrappedMongoDriverInformation, settings, syncStreamFactoryFactory); + delegate = new MongoClientImpl(cluster, wrappedMongoDriverInformation, settings, syncStreamFactoryFactory, clientExecutor); this.options = options != null ? options : MongoClientOptions.builder(settings).build(); cursorCleaningService = this.options.isCursorFinalizerEnabled() ? createCursorCleaningService() : null; this.closed = new AtomicBoolean(); diff --git a/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy b/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy index f6a51e5607c..11de3f9d7dd 100644 --- a/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy +++ b/driver-legacy/src/test/unit/com/mongodb/MongoClientSpecification.groovy @@ -38,6 +38,7 @@ import static com.mongodb.CustomMatchers.isTheSameAs import static com.mongodb.MongoClientSettings.getDefaultCodecRegistry import static com.mongodb.MongoCredential.createMongoX509Credential import static com.mongodb.ReadPreference.secondary +import static com.mongodb.assertions.Assertions.fail import static com.mongodb.connection.ClusterConnectionMode.MULTIPLE import static com.mongodb.connection.ClusterConnectionMode.SINGLE import static java.util.concurrent.TimeUnit.MILLISECONDS @@ -316,7 +317,9 @@ class MongoClientSpecification extends Specification { def clusterStub = Stub(Cluster) clusterStub.getClientMetadata() >> new ClientMetadata("test", MongoDriverInformation.builder().build()) - def client = new MongoClientImpl(clusterStub, null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), executor) + def client = new MongoClientImpl( + clusterStub, null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), + AsyncClientExecutor.NO_OP, executor) when: client.watch((Class) null) @@ -370,7 +373,9 @@ class MongoClientSpecification extends Specification { def mockStreamFactoryFactory() { Mock(StreamFactoryFactory) { - getClientExecutor() >> AsyncClientExecutor.NO_OP + getExecutor() >> { + fail() + } } } } diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java index 922ca40b121..8c23eece19f 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoClients.java @@ -119,8 +119,9 @@ public static MongoClient create(final MongoClientSettings settings, @Nullable f StreamFactory streamFactory = getStreamFactory(streamFactoryFactory, settings, false); StreamFactory heartbeatStreamFactory = getStreamFactory(streamFactoryFactory, settings, true); MongoDriverInformation wrappedMongoDriverInformation = wrapMongoDriverInformation(mongoDriverInformation); - Cluster cluster = createCluster(settings, wrappedMongoDriverInformation, streamFactory, heartbeatStreamFactory, streamFactoryFactory.getClientExecutor()); - return new MongoClientImpl(cluster, wrappedMongoDriverInformation, settings, streamFactoryFactory); + AsyncClientExecutor clientExecutor = AsyncClientExecutor.backedBy(streamFactoryFactory.getExecutor()); + Cluster cluster = createCluster(settings, wrappedMongoDriverInformation, streamFactory, heartbeatStreamFactory, clientExecutor); + return new MongoClientImpl(cluster, wrappedMongoDriverInformation, settings, streamFactoryFactory, clientExecutor); } /** diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java index 9b803e222f2..64402d4b254 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/MongoClientImpl.java @@ -74,6 +74,7 @@ public final class MongoClientImpl implements MongoClient { private static final Logger LOGGER = Loggers.getLogger("client"); private final MongoClientSettings settings; private final StreamFactoryFactory streamFactoryFactory; + private final AsyncClientExecutor clientExecutor; private final MongoClusterImpl delegate; private final AtomicBoolean closed; @@ -82,8 +83,9 @@ public MongoClientImpl( final Cluster cluster, final MongoDriverInformation mongoDriverInformation, final MongoClientSettings settings, - final StreamFactoryFactory streamFactoryFactory) { - this(cluster, mongoDriverInformation, settings, streamFactoryFactory, null); + final StreamFactoryFactory streamFactoryFactory, + final AsyncClientExecutor clientExecutor) { + this(cluster, mongoDriverInformation, settings, streamFactoryFactory, clientExecutor, null); } @VisibleForTesting(otherwise = PRIVATE) @@ -92,13 +94,14 @@ public MongoClientImpl( final MongoDriverInformation mongoDriverInformation, final MongoClientSettings settings, final StreamFactoryFactory streamFactoryFactory, + final AsyncClientExecutor clientExecutor, @Nullable final OperationExecutor executor) { notNull("settings", settings); notNull("cluster", cluster); TracingManager tracingManager = new TracingManager(settings.getObservabilitySettings()); TimeoutSettings timeoutSettings = TimeoutSettings.create(settings); - ServerSessionPool serverSessionPool = new ServerSessionPool(cluster, streamFactoryFactory.getClientExecutor(), timeoutSettings, settings.getServerApi()); + ServerSessionPool serverSessionPool = new ServerSessionPool(cluster, clientExecutor, timeoutSettings, settings.getServerApi()); ClientSessionHelper clientSessionHelper = new ClientSessionHelper(this, serverSessionPool, tracingManager); AutoEncryptionSettings autoEncryptSettings = settings.getAutoEncryptionSettings(); @@ -125,6 +128,7 @@ public MongoClientImpl( this.delegate = new MongoClusterImpl(cluster, crypt, operationExecutor, serverSessionPool, clientSessionHelper, mongoOperationPublisher); this.streamFactoryFactory = streamFactoryFactory; + this.clientExecutor = clientExecutor; this.settings = settings; this.closed = new AtomicBoolean(); @@ -162,8 +166,11 @@ public void close() { } getServerSessionPool().close(); getCluster().close(); - try { - streamFactoryFactory.close(); + //noinspection EmptyTryBlock + try (AutoCloseable autoClosedStreamFactoryFactory = streamFactoryFactory; + AutoCloseable autoClosedClientExecutor = clientExecutor) { + // `clientExecutor`, `streamFactoryFactory` must be the last resources closed, + // with `streamFactoryFactory` being the very last. } catch (Exception e) { LOGGER.warn("Exception closing resource", e); } @@ -342,10 +349,7 @@ public void appendMetadata(final MongoDriverInformation mongoDriverInformation) LOGGER.info(format("MongoClient metadata has been updated to %s", clientMetadata.getBsonDocument())); } - /** - * @see StreamFactoryFactory#getClientExecutor() - */ public AsyncClientExecutor getClientExecutor() { - return streamFactoryFactory.getClientExecutor(); + return clientExecutor; } } diff --git a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java index 490bdce7ad4..fc9f24506aa 100644 --- a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java +++ b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/MongoClientImplTest.java @@ -204,15 +204,25 @@ void testStartSession() { }); } + @Test + void close() { + com.mongodb.client.MongoClientTest.assertClose((cluster, mongoDriverInformation, streamFactoryFactory, clientExecutor) -> + new MongoClientImpl( + cluster, + mongoDriverInformation, + MongoClientSettings.builder().build(), + streamFactoryFactory, + clientExecutor)); + } + private MongoClientImpl createMongoClient() { MongoDriverInformation mongoDriverInformation = MongoDriverInformation.builder().driverName("reactive-streams").build(); Cluster cluster = MongoMockito.mock(Cluster.class, mock -> { when(mock.getClientMetadata()) .thenReturn(new ClientMetadata("test", mongoDriverInformation)); }); - StreamFactoryFactory streamFactoryFactory = MongoMockito.mock(StreamFactoryFactory.class, mock -> { - when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.NO_OP); - }); - return new MongoClientImpl(cluster, mongoDriverInformation, MongoClientSettings.builder().build(), streamFactoryFactory, OPERATION_EXECUTOR); + StreamFactoryFactory streamFactoryFactory = MongoMockito.mock(StreamFactoryFactory.class); + return new MongoClientImpl(cluster, mongoDriverInformation, MongoClientSettings.builder().build(), streamFactoryFactory, + AsyncClientExecutor.NO_OP, OPERATION_EXECUTOR); } } diff --git a/driver-sync/src/main/com/mongodb/client/MongoClients.java b/driver-sync/src/main/com/mongodb/client/MongoClients.java index 9ce2e87ea70..ccdb30be46f 100644 --- a/driver-sync/src/main/com/mongodb/client/MongoClients.java +++ b/driver-sync/src/main/com/mongodb/client/MongoClients.java @@ -23,6 +23,7 @@ import com.mongodb.client.internal.MongoClientImpl; import com.mongodb.internal.connection.Cluster; import com.mongodb.internal.connection.StreamFactoryFactory; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import static com.mongodb.assertions.Assertions.notNull; @@ -120,13 +121,14 @@ public static MongoClient create(final MongoClientSettings settings, @Nullable f StreamFactoryFactory syncStreamFactoryFactory = getSyncStreamFactoryFactory( settings.getTransportSettings(), getInetAddressResolver(settings)); - + AsyncClientExecutor clientExecutor = AsyncClientExecutor.NO_OP; Cluster cluster = Clusters.createCluster( settings, driverInfo, - syncStreamFactoryFactory); + syncStreamFactoryFactory, + clientExecutor); - return new MongoClientImpl(cluster, driverInfo, settings, syncStreamFactoryFactory); + return new MongoClientImpl(cluster, driverInfo, settings, syncStreamFactoryFactory, clientExecutor); } private MongoClients() { diff --git a/driver-sync/src/main/com/mongodb/client/internal/Clusters.java b/driver-sync/src/main/com/mongodb/client/internal/Clusters.java index ffedcfbace7..88cbebaa561 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/Clusters.java +++ b/driver-sync/src/main/com/mongodb/client/internal/Clusters.java @@ -24,6 +24,7 @@ import com.mongodb.internal.connection.InternalConnectionPoolSettings; import com.mongodb.internal.connection.StreamFactory; import com.mongodb.internal.connection.StreamFactoryFactory; +import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; import static com.mongodb.assertions.Assertions.assertNotNull; @@ -37,7 +38,8 @@ private Clusters() { public static Cluster createCluster(final MongoClientSettings settings, @Nullable final MongoDriverInformation mongoDriverInformation, - final StreamFactoryFactory streamFactoryFactory) { + final StreamFactoryFactory streamFactoryFactory, + final AsyncClientExecutor clientExecutor) { assertNotNull(streamFactoryFactory); assertNotNull(settings); @@ -47,7 +49,7 @@ public static Cluster createCluster(final MongoClientSettings settings, return new DefaultClusterFactory().createCluster(settings.getClusterSettings(), settings.getServerSettings(), settings.getConnectionPoolSettings(), InternalConnectionPoolSettings.builder().build(), TimeoutSettings.create(settings), streamFactory, - TimeoutSettings.createHeartbeatSettings(settings), heartbeatStreamFactory, streamFactoryFactory.getClientExecutor(), + TimeoutSettings.createHeartbeatSettings(settings), heartbeatStreamFactory, clientExecutor, settings.getCredential(), settings.getLoggerSettings(), getCommandListener(settings.getCommandListeners()), settings.getApplicationName(), mongoDriverInformation, settings.getCompressorList(), settings.getServerApi(), settings.getDnsClient()); diff --git a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java index 73e4ead5388..fac6709c677 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/MongoClientImpl.java @@ -73,13 +73,15 @@ public final class MongoClientImpl implements MongoClient { private final MongoClusterImpl delegate; private final AtomicBoolean closed; private final StreamFactoryFactory streamFactoryFactory; + private final AsyncClientExecutor clientExecutor; public MongoClientImpl( final Cluster cluster, final MongoDriverInformation mongoDriverInformation, final MongoClientSettings settings, - final StreamFactoryFactory streamFactoryFactory) { - this(cluster, mongoDriverInformation, settings, streamFactoryFactory, null); + final StreamFactoryFactory streamFactoryFactory, + final AsyncClientExecutor clientExecutor) { + this(cluster, mongoDriverInformation, settings, streamFactoryFactory, clientExecutor, null); } @VisibleForTesting(otherwise = PRIVATE) @@ -88,9 +90,11 @@ public MongoClientImpl( final MongoDriverInformation mongoDriverInformation, final MongoClientSettings settings, final StreamFactoryFactory streamFactoryFactory, + final AsyncClientExecutor clientExecutor, @Nullable final OperationExecutor operationExecutor) { this.streamFactoryFactory = streamFactoryFactory; + this.clientExecutor = clientExecutor; this.settings = notNull("settings", settings); this.mongoDriverInformation = mongoDriverInformation; AutoEncryptionSettings autoEncryptionSettings = settings.getAutoEncryptionSettings(); @@ -99,7 +103,6 @@ public MongoClientImpl( + SynchronousContextProvider.class.getName() + " when using the synchronous driver"); } - AsyncClientExecutor clientExecutor = streamFactoryFactory.getClientExecutor(); this.delegate = new MongoClusterImpl(autoEncryptionSettings, cluster, withUuidRepresentation(settings.getCodecRegistry(), settings.getUuidRepresentation()), (SynchronousContextProvider) settings.getContextProvider(), @@ -125,8 +128,11 @@ public void close() { } delegate.getServerSessionPool().close(); delegate.getCluster().close(); - try { - streamFactoryFactory.close(); + //noinspection EmptyTryBlock + try (AutoCloseable autoClosedStreamFactoryFactory = streamFactoryFactory; + AutoCloseable autoClosedClientExecutor = clientExecutor) { + // `clientExecutor`, `streamFactoryFactory` must be the last resources closed, + // with `streamFactoryFactory` being the very last. } catch (Exception e) { LOGGER.warn("Exception closing resource", e); } @@ -334,10 +340,7 @@ public MongoDriverInformation getMongoDriverInformation() { return mongoDriverInformation; } - /** - * @see StreamFactoryFactory#getClientExecutor() - */ public AsyncClientExecutor getClientExecutor() { - return streamFactoryFactory.getClientExecutor(); + return clientExecutor; } } diff --git a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java index 3565ad6c7d5..c7912a25741 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/MongoClientTest.java @@ -29,6 +29,7 @@ import com.mongodb.internal.mockito.MongoMockito; import com.mongodb.internal.thread.AsyncClientExecutor; import org.junit.jupiter.api.Test; +import org.mockito.InOrder; import org.mockito.Mockito; import java.util.concurrent.CompletableFuture; @@ -37,11 +38,12 @@ import java.util.concurrent.TimeoutException; import static com.mongodb.client.Fixture.getMongoClientSettingsBuilder; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.when; -class MongoClientTest { +public final class MongoClientTest { @SuppressWarnings("try") @Test @@ -65,39 +67,45 @@ public void clusterOpening(final ClusterOpeningEvent event) { } @Test - void shouldCloseStreamFactoryFactory() { + void close() { + assertClose((cluster, mongoDriverInformation, streamFactoryFactory, clientExecutor) -> + new MongoClientImpl( + cluster, + mongoDriverInformation, + MongoClientSettings.builder().build(), + streamFactoryFactory, + clientExecutor)); + } - //given + public static void assertClose(final MongoClientCreator clientCreator) { MongoDriverInformation mongoDriverInformation = MongoDriverInformation.builder().build(); - Cluster cluster = MongoMockito.mock( - Cluster.class, - mockedCluster -> { - doNothing().when(mockedCluster).close(); - when(mockedCluster.getClientMetadata()) - .thenReturn(new ClientMetadata("test", mongoDriverInformation)); - }); - StreamFactoryFactory streamFactoryFactory = MongoMockito.mock( - StreamFactoryFactory.class, - mock -> { - when(mock.getClientExecutor()).thenReturn(AsyncClientExecutor.NO_OP); - try { - doNothing().when(mock).close(); - } catch (Exception e) { - throw new RuntimeException(e); - } - }); - - MongoClientImpl mongoClient = new MongoClientImpl( + Cluster cluster = MongoMockito.mock(Cluster.class, mock -> { + doNothing().when(mock).close(); + when(mock.getClientMetadata()).thenReturn(new ClientMetadata("test", mongoDriverInformation)); + }); + StreamFactoryFactory streamFactoryFactory = MongoMockito.mock(StreamFactoryFactory.class, mock -> { + doNothing().when(mock).close(); + }); + AsyncClientExecutor clientExecutor = MongoMockito.mock(AsyncClientExecutor.class, mock -> { + doNothing().when(mock).close(); + }); + AutoCloseable mongoClient = clientCreator.create( cluster, mongoDriverInformation, - MongoClientSettings.builder().build(), - streamFactoryFactory); - - //when - mongoClient.close(); + streamFactoryFactory, + clientExecutor); + assertDoesNotThrow(() -> mongoClient.close()); + InOrder inOrder = Mockito.inOrder(cluster, clientExecutor, streamFactoryFactory); + inOrder.verify(cluster).close(); + inOrder.verify(clientExecutor).close(); + inOrder.verify(streamFactoryFactory).close(); + } - //then - Mockito.verify(streamFactoryFactory).close(); - Mockito.verify(cluster).close(); + public interface MongoClientCreator { + AutoCloseable create( + Cluster cluster, + MongoDriverInformation mongoDriverInformation, + StreamFactoryFactory streamFactoryFactory, + AsyncClientExecutor clientExecutor); } } diff --git a/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy b/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy index f8a825b96ab..cf1dcdcd2a3 100644 --- a/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy +++ b/driver-sync/src/test/unit/com/mongodb/client/MongoClientSpecification.groovy @@ -50,6 +50,7 @@ import static com.mongodb.CustomMatchers.isTheSameAs import static com.mongodb.MongoClientSettings.getDefaultCodecRegistry import static com.mongodb.ReadPreference.primary import static com.mongodb.ReadPreference.secondary +import static com.mongodb.assertions.Assertions.fail import static com.mongodb.client.internal.TestHelper.execute import static java.util.concurrent.TimeUnit.SECONDS import static org.bson.UuidRepresentation.C_SHARP_LEGACY @@ -72,7 +73,8 @@ class MongoClientSpecification extends Specification { .retryWrites(true) .codecRegistry(CODEC_REGISTRY) .build() - def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), new TestOperationExecutor([])) + def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), + AsyncClientExecutor.NO_OP, new TestOperationExecutor([])) when: def database = client.getDatabase('name') @@ -89,7 +91,8 @@ class MongoClientSpecification extends Specification { def 'should use ListDatabasesIterableImpl correctly'() { given: def executor = new TestOperationExecutor([null, null]) - def client = new MongoClientImpl(Stub(Cluster), null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), executor) + def client = new MongoClientImpl(Stub(Cluster), null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), + AsyncClientExecutor.NO_OP, executor) def listDatabasesMethod = client.&listDatabases def listDatabasesNamesMethod = client.&listDatabaseNames @@ -134,7 +137,8 @@ class MongoClientSpecification extends Specification { .build() def readPreference = settings.getReadPreference() def readConcern = settings.getReadConcern() - def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), executor) + def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), + AsyncClientExecutor.NO_OP, executor) def watchMethod = client.&watch when: @@ -172,7 +176,7 @@ class MongoClientSpecification extends Specification { given: def executor = new TestOperationExecutor([]) def client = new MongoClientImpl(Stub(Cluster), null, MongoClientSettings.builder().build(), mockStreamFactoryFactory(), - executor) + AsyncClientExecutor.NO_OP, executor) when: client.watch((Class) null) @@ -203,7 +207,8 @@ class MongoClientSpecification extends Specification { 1 * getClientMetadata() >> new ClientMetadata("test", driverInformation) } def settings = MongoClientSettings.builder().build() - def client = new MongoClientImpl(cluster, driverInformation, settings, mockStreamFactoryFactory(), new TestOperationExecutor([])) + def client = new MongoClientImpl(cluster, driverInformation, settings, mockStreamFactoryFactory(), + AsyncClientExecutor.NO_OP, new TestOperationExecutor([])) expect: client.getClusterDescription() == clusterDescription @@ -218,7 +223,8 @@ class MongoClientSpecification extends Specification { .build() when: - def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), new TestOperationExecutor([])) + def client = new MongoClientImpl(Stub(Cluster), null, settings, mockStreamFactoryFactory(), + AsyncClientExecutor.NO_OP, new TestOperationExecutor([])) then: (client.getCodecRegistry().get(UUID) as UuidCodec).getUuidRepresentation() == C_SHARP_LEGACY @@ -229,7 +235,9 @@ class MongoClientSpecification extends Specification { def mockStreamFactoryFactory() { Mock(StreamFactoryFactory) { - getClientExecutor() >> AsyncClientExecutor.NO_OP + getExecutor() >> { + fail() + } } } } From f0a80c1d02de83797199fb235c90983ab802fdfd Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Thu, 13 Aug 2026 07:11:27 -0600 Subject: [PATCH 11/14] Rename `UnimplementedAsyncClientExecutor` -> `NoOpAsyncClientExecutor` --- .../internal/thread/AsyncClientExecutor.java | 2 +- ...ntExecutor.java => NoOpAsyncClientExecutor.java} | 13 +++++-------- ...orTest.java => NoOpAsyncClientExecutorTest.java} | 13 ++++++++++--- 3 files changed, 16 insertions(+), 12 deletions(-) rename driver-core/src/main/com/mongodb/internal/thread/{UnimplementedAsyncClientExecutor.java => NoOpAsyncClientExecutor.java} (79%) rename driver-core/src/test/unit/com/mongodb/internal/thread/{UnimplementedAsyncClientExecutorTest.java => NoOpAsyncClientExecutorTest.java} (84%) diff --git a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java index 319074e44f5..3059e9665cb 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java @@ -43,7 +43,7 @@ */ @ThreadSafe public interface AsyncClientExecutor extends AutoCloseable { - AsyncClientExecutor NO_OP = UnimplementedAsyncClientExecutor.instance(); + AsyncClientExecutor NO_OP = new NoOpAsyncClientExecutor(); /** * @param executor The executor to use for executing tasks. diff --git a/driver-core/src/main/com/mongodb/internal/thread/UnimplementedAsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/NoOpAsyncClientExecutor.java similarity index 79% rename from driver-core/src/main/com/mongodb/internal/thread/UnimplementedAsyncClientExecutor.java rename to driver-core/src/main/com/mongodb/internal/thread/NoOpAsyncClientExecutor.java index 8213fe47d73..e2ab719e233 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/UnimplementedAsyncClientExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/NoOpAsyncClientExecutor.java @@ -22,15 +22,12 @@ import static com.mongodb.assertions.Assertions.fail; +/** + * The only instance of this class is {@link AsyncClientExecutor#NO_OP}. + */ @ThreadSafe -final class UnimplementedAsyncClientExecutor implements AsyncClientExecutor { - private static final UnimplementedAsyncClientExecutor INSTANCE = new UnimplementedAsyncClientExecutor(); - - static UnimplementedAsyncClientExecutor instance() { - return INSTANCE; - } - - private UnimplementedAsyncClientExecutor() { +final class NoOpAsyncClientExecutor implements AsyncClientExecutor { + NoOpAsyncClientExecutor() { } /** diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/UnimplementedAsyncClientExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/NoOpAsyncClientExecutorTest.java similarity index 84% rename from driver-core/src/test/unit/com/mongodb/internal/thread/UnimplementedAsyncClientExecutorTest.java rename to driver-core/src/test/unit/com/mongodb/internal/thread/NoOpAsyncClientExecutorTest.java index 632e92a29d7..064fecbe0b4 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/thread/UnimplementedAsyncClientExecutorTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/NoOpAsyncClientExecutorTest.java @@ -15,6 +15,7 @@ */ package com.mongodb.internal.thread; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -23,10 +24,16 @@ import java.util.concurrent.CompletionException; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; -class UnimplementedAsyncClientExecutorTest { +class NoOpAsyncClientExecutorTest { + @Test + void noOpConstantIsOfTheRightType() { + assertInstanceOf(NoOpAsyncClientExecutor.class, AsyncClientExecutor.NO_OP); + } + @ParameterizedTest @ValueSource(longs = {0, 200}) void sleepAsync(final long durationMs) { @@ -37,8 +44,8 @@ void sleepAsync(final long durationMs) { private static void assertSleepAsync(final Duration duration) { CompletableFuture callbackExceptionFuture = new CompletableFuture<>(); CompletableFuture callbackThreadFuture = new CompletableFuture<>(); - try (UnimplementedAsyncClientExecutor unimplementedClientExecutor = UnimplementedAsyncClientExecutor.instance()) { - unimplementedClientExecutor.sleepAsync(duration, (result, t) -> { + try (AsyncClientExecutor noOpClientExecutor = AsyncClientExecutor.NO_OP) { + noOpClientExecutor.sleepAsync(duration, (result, t) -> { if (t != null) { callbackExceptionFuture.completeExceptionally(t); } else { From b6948c9462f6023c9a1822cc337725f60d15332b Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Sat, 15 Aug 2026 10:24:17 -0600 Subject: [PATCH 12/14] Refactor `AsyncClientExecutor.sleepAsync`->`schedule`, add `ThreadUtil.sleepAsync` --- .../RetryingAsyncCallbackSupplier.java | 6 +- .../async/function/RetryingSyncSupplier.java | 12 +- .../internal/thread/AsyncClientExecutor.java | 53 ++++-- .../internal/thread/CommonExecutor.java | 8 +- .../thread/DefaultAsyncClientExecutor.java | 159 +++++++++--------- .../thread/NoOpAsyncClientExecutor.java | 10 +- .../mongodb/internal/thread/ThreadUtil.java | 67 ++++++++ .../DefaultAsyncClientExecutorTest.java | 149 ++++++++++------ .../thread/NoOpAsyncClientExecutorTest.java | 66 -------- .../internal/thread/ThreadUtilTest.java | 66 ++++++++ 10 files changed, 365 insertions(+), 231 deletions(-) create mode 100644 driver-core/src/main/com/mongodb/internal/thread/ThreadUtil.java delete mode 100644 driver-core/src/test/unit/com/mongodb/internal/thread/NoOpAsyncClientExecutorTest.java create mode 100644 driver-core/src/test/unit/com/mongodb/internal/thread/ThreadUtilTest.java diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java index 0a4c5761757..3b229ed20da 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingAsyncCallbackSupplier.java @@ -20,10 +20,12 @@ import com.mongodb.internal.async.SingleResultCallback; import com.mongodb.internal.async.function.RetryPolicy.Decision.RetryAttemptInfo; import com.mongodb.internal.thread.AsyncClientExecutor; +import com.mongodb.internal.thread.ThreadUtil; import java.time.Duration; import static com.mongodb.internal.async.AsyncRunnable.beginAsync; +import static com.mongodb.internal.thread.ThreadUtil.sleepAsync; /** * A decorator that implements automatic retrying of failed executions of an {@link AsyncCallbackSupplier}. @@ -43,7 +45,7 @@ public final class RetryingAsyncCallbackSupplier implements AsyncCallbackSupp private final AsyncCallbackSupplier asyncFunction; /** - * @param clientExecutor For {@linkplain AsyncClientExecutor#sleepAsync(Duration, SingleResultCallback) delaying} attempts + * @param clientExecutor For {@linkplain ThreadUtil#sleepAsync(Duration, AsyncClientExecutor, SingleResultCallback) delaying} attempts * according to {@link RetryAttemptInfo#getBackoff()}. * @param control The {@link RetryControl} to control the new {@link RetryingAsyncCallbackSupplier}. * @param asyncFunction The retryable {@link AsyncCallbackSupplier} to be decorated. @@ -72,7 +74,7 @@ public void get(final SingleResultCallback callback) { onAttemptFailureCallback.completeExceptionally(attemptFailedResult); } else { RetryAttemptInfo retryAttemptInfo = control.advanceOrThrow(attemptFailedResult); - clientExecutor.sleepAsync(retryAttemptInfo.getBackoff(), onAttemptFailureCallback); + sleepAsync(retryAttemptInfo.getBackoff(), clientExecutor, onAttemptFailureCallback); } }).finish(iterationCallback); }).thenSupply(c -> { diff --git a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java index 75c616e88da..89cb2d0c8b8 100644 --- a/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java +++ b/driver-core/src/main/com/mongodb/internal/async/function/RetryingSyncSupplier.java @@ -20,11 +20,9 @@ import com.mongodb.internal.thread.AsyncClientExecutor; import com.mongodb.lang.Nullable; -import java.time.Duration; import java.util.function.Supplier; -import static com.mongodb.internal.thread.InterruptionUtil.interruptAndCreateMongoInterruptedException; -import static java.util.concurrent.TimeUnit.NANOSECONDS; +import static com.mongodb.internal.thread.ThreadUtil.sleep; /** * A decorator that implements automatic retrying of failed executions of a {@link Supplier}. @@ -63,12 +61,4 @@ public R get() { } } } - - private static void sleep(final Duration duration) { - try { - NANOSECONDS.sleep(duration.toNanos()); - } catch (InterruptedException e) { - throw interruptAndCreateMongoInterruptedException(null, e); - } - } } diff --git a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java index 3059e9665cb..cfca7fa48c9 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/AsyncClientExecutor.java @@ -23,7 +23,9 @@ import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; /** * An executor for a {@code MongoClient} (currently, only for an asynchronous one). @@ -56,24 +58,51 @@ static AsyncClientExecutor backedBy(final Executor executor) { } /** - * The callback-based counterpart to {@link Thread#sleep(long, int)}. - * - * @param duration A non-{@linkplain Duration#isNegative() negative} duration. - * If {@code duration} is {@linkplain Duration#isZero() zero}, - * the {@code callback} is {@linkplain SingleResultCallback#complete(SingleResultCallback) completed} - * by the {@link Thread} that invoked the method. - * Regardless of the {@link Duration}, {@link #close()} causes the {@code callback} to be completed with - * {@link RejectedExecutionException}. + * @param task The task to execute. It may not be executed if this method completes abruptly, but is + * {@linkplain RejectableRunnable#reject(RejectedExecutionException) executed} if the executor is {@linkplain #close() closed}. + * @param delay A non-{@linkplain Duration#isNegative() negative} duration. */ - void sleepAsync(Duration duration, SingleResultCallback callback); + void schedule(RejectableRunnable task, Duration delay); /** * Must be called before shutting down the {@linkplain #backedBy(Executor) backing executor}, * to notify this {@link AsyncClientExecutor} that the backing executor may be about to shut down. - * This method guarantees exactly-once {@linkplain SingleResultCallback#onResult(Object, Throwable) completion} - * of all callbacks that may have not been completed otherwise if the backing executor shuts down. - * An example of such a callback is one scheduled via {@link #sleepAsync(Duration, SingleResultCallback)}. + * Guarantees exactly-once execution of all {@link RejectableRunnable} tasks, + * which may have not been executed otherwise if the backing executor shuts down. */ @Override void close(); + + /** + * Either {@link #run()} or {@link #reject(RejectedExecutionException)} may be executed, but never both. + * Executing either means executing this {@link RejectableRunnable}. + */ + interface RejectableRunnable extends Runnable { + /** + * Unlike {@link ScheduledThreadPoolExecutor}, which handles rejections based on the {@link RejectedExecutionHandler}, + * and by default throws {@link RejectedExecutionException}, + * {@link AsyncClientExecutor} delegates rejection handling to the scheduled tasks by invoking this method. + * This way, {@link #close()} does not have to return the list of tasks that never commenced execution, + * and its caller does not have to deal with them to make sure all the tasks are executed in some way + * (if a task must complete a callback, failing to execute it is a critical bug). + * Additionally, this method allows reacting to rejection differently than what {@link #run()} would have done. + * + * @see #close() + */ + void reject(RejectedExecutionException cause); + + static RejectableRunnable from(final SingleResultCallback callback) { + return new RejectableRunnable() { + @Override + public void reject(final RejectedExecutionException cause) { + callback.completeExceptionally(cause); + } + + @Override + public void run() { + callback.complete(callback); + } + }; + } + } } diff --git a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java index 0386073c7d8..16dbac39d72 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java @@ -27,6 +27,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import static com.mongodb.assertions.Assertions.assertFalse; import static com.mongodb.assertions.Assertions.fail; import static java.lang.String.format; import static java.util.concurrent.TimeUnit.NANOSECONDS; @@ -60,9 +61,11 @@ private CommonExecutor() { } /** - * @param task The task to be scheduled. If it is {@link Executor#execute(Runnable) executed}, then the execution is guaranteed + * @param task The task to be scheduled. It may not be executed if this method completes abruptly. + * If it is {@link Executor#execute(Runnable) executed}, then the execution is guaranteed * to be done via the {@code executor}. However, if the {@code executor} is shut down, then it does not execute the {@code task}, - * and there is nothing we can do about that. + * and there is nothing {@link CommonExecutor} can do about that. The caller, on the other hand, may deal with such a situation, + * like {@link AsyncClientExecutor#schedule(AsyncClientExecutor.RejectableRunnable, Duration)} does, for example. * @param delay A non-{@linkplain Duration#isNegative() negative} delay. * @param executor The {@link Executor} to use for {@code task} {@linkplain Runnable#run() execution}, * so that the {@code task} is not executed by a thread managed by {@link CommonExecutor}. @@ -71,6 +74,7 @@ private CommonExecutor() { * and not the execution part done by the {@code executor}. */ ScheduledFuture schedule(final Runnable task, final Duration delay, final Executor executor) { + assertFalse(delay.isNegative()); try { return singleThreadScheduler.schedule( () -> { diff --git a/driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java index bf70de0bed0..c52ff89962b 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/DefaultAsyncClientExecutor.java @@ -17,7 +17,6 @@ import com.mongodb.annotations.NotThreadSafe; import com.mongodb.annotations.ThreadSafe; -import com.mongodb.internal.async.SingleResultCallback; import com.mongodb.lang.Nullable; import java.time.Duration; @@ -35,7 +34,6 @@ import static com.mongodb.assertions.Assertions.assertFalse; import static com.mongodb.assertions.Assertions.assertNull; import static com.mongodb.internal.Locks.withLock; -import static com.mongodb.internal.async.AsyncRunnable.beginAsync; import static com.mongodb.internal.thread.CommonExecutor.commonExecutor; import static java.lang.String.format; import static java.util.concurrent.TimeUnit.NANOSECONDS; @@ -43,11 +41,11 @@ @ThreadSafe final class DefaultAsyncClientExecutor implements AsyncClientExecutor { private final Executor backingExecutor; - private final Set scheduledCallbackCompletions; + private final Set scheduledTasks; /** * While holding this lock, no application code may be executed, and driver code external to {@link DefaultAsyncClientExecutor} * should be either avoided or carefully vetted. This is to avoid unexpected delays and deadlocks. - * For example, {@link ScheduledCallbackCompletion#reject(RejectedExecutionException)}, {@link ScheduledCallbackCompletion#run()} + * For example, {@link ScheduledRejectableRunnable#reject(RejectedExecutionException)}, {@link ScheduledRejectableRunnable#run()} * must not be executed while holding the lock. */ private final ReentrantLock closeLock; @@ -55,112 +53,112 @@ final class DefaultAsyncClientExecutor implements AsyncClientExecutor { DefaultAsyncClientExecutor(final Executor backingExecutor) { this.backingExecutor = backingExecutor; - scheduledCallbackCompletions = ConcurrentHashMap.newKeySet(); + scheduledTasks = ConcurrentHashMap.newKeySet(); closeLock = new ReentrantLock(); closed = false; } @Override - public void sleepAsync(final Duration duration, final SingleResultCallback callback) { - sleepAsync(closed, duration, callback, () -> scheduleCompletion(callback, duration)); + public void schedule(final RejectableRunnable task, final Duration delay) { + assertFalse(delay.isNegative()); + ScheduledRejectableRunnable scheduledTask = new ScheduledRejectableRunnable(task); + try { + withLock(closeLock, () -> { + scheduledTasks.add(scheduledTask); + if (closed) { + throw createClosedException(); + } + // We handle `backingExecutor.isShutdown` and `RejectedExecutionException` from `backingExecutor.schedule` + // merely as the best effort to improve the application experience. + // Either situation violates the contract of the `close` method. + if (backingExecutor instanceof ExecutorService && ((ExecutorService) backingExecutor).isShutdown()) { + throw createBackingExecutorShutdownException(); + } + ScheduledFuture scheduledFuture = (backingExecutor instanceof ScheduledExecutorService) + ? ((ScheduledExecutorService) backingExecutor).schedule(scheduledTask, delay.toNanos(), NANOSECONDS) + : commonExecutor().schedule(scheduledTask, delay, backingExecutor); + scheduledTask.onScheduled(scheduledFuture); + }); + } catch (RejectedExecutionException rejectionCause) { + scheduledTask.reject(rejectionCause); + } } - /** - * @param closed See {@link #closed}. - * @param onPositiveDurationDelayAndCompleteCallback Runs only if - * {@code duration} is {@linkplain Duration#isNegative() positive}. - * The {@link Runnable#run()} is allowed to complete abruptly and is guaranteed to run in the same thread that invoked this method. - */ - static void sleepAsync( - final boolean closed, - final Duration duration, - final SingleResultCallback callback, - final Runnable onPositiveDurationDelayAndCompleteCallback) { - beginAsync().thenRun(c -> { - assertFalse(duration.isNegative()); - if (closed) { - throw createClosedException(); - } - if (duration.isZero()) { - c.complete(c); - } else { - onPositiveDurationDelayAndCompleteCallback.run(); - } - }).finish(callback); + private static RejectedExecutionException createClosedException() { + return new RejectedExecutionException("Closed"); } - private void scheduleCompletion(final SingleResultCallback callback, final Duration delay) { - ScheduledCallbackCompletion scheduledCallbackCompletion = new ScheduledCallbackCompletion(callback); - RejectedExecutionException rejectionCause = withLock(closeLock, () -> { - scheduledCallbackCompletions.add(scheduledCallbackCompletion); - if (closed) { - return createClosedException(); - } - ScheduledFuture scheduledFuture; - // We handle `isShutdown`, `RejectedExecutionException` below merely - // as the best effort to improve the application experience. - // Either situation violates the contract of the `close` method. - if (backingExecutor instanceof ExecutorService && ((ExecutorService) backingExecutor).isShutdown()) { - return createBackingExecutorShutdownException(); - } - if (backingExecutor instanceof ScheduledExecutorService) { - try { - scheduledFuture = ((ScheduledExecutorService) backingExecutor).schedule(scheduledCallbackCompletion, delay.toNanos(), NANOSECONDS); - } catch (RejectedExecutionException rejected) { - return rejected; - } - } else { - scheduledFuture = commonExecutor().schedule(scheduledCallbackCompletion, delay, backingExecutor); - } - scheduledCallbackCompletion.onScheduled(scheduledFuture); - return null; - }); - if (rejectionCause != null) { - scheduledCallbackCompletion.reject(rejectionCause); - } + private RejectedExecutionException createBackingExecutorShutdownException() { + return new RejectedExecutionException(format("The backing executor %s is shut down", backingExecutor)); } @Override public void close() { - Collection localScheduledCallbackCompletions = new ArrayList<>(); + Collection localScheduledTasks = new ArrayList<>(); withLock(closeLock, () -> { if (closed) { return; } closed = true; - // Here we do not care about any `ScheduledCallbackCompletion` added after the current critical section, + // Here we do not care about any `ScheduledRejectableRunnable` added after the current critical section, // because its `reject` is called by the method that added it. - localScheduledCallbackCompletions.addAll(scheduledCallbackCompletions); + localScheduledTasks.addAll(scheduledTasks); }); - for (ScheduledCallbackCompletion scheduledCallbackCompletion : localScheduledCallbackCompletions) { - scheduledCallbackCompletion.reject(createClosedException()); + Throwable primaryException = null; + try { + for (ScheduledRejectableRunnable scheduledTask : localScheduledTasks) { + try { + scheduledTask.reject(createClosedException()); + } catch (Throwable t) { + primaryException = suppressUnlessThereIsNoPrimary(primaryException, t); + } + } + } catch (Throwable t) { + primaryException = suppressUnlessThereIsNoPrimary(primaryException, t); + } finally { + rethrowAsUnchecked(primaryException); } } - private static RejectedExecutionException createClosedException() { - return new RejectedExecutionException("Closed"); + private static Throwable suppressUnlessThereIsNoPrimary( + @Nullable final Throwable maybePrimary, + final Throwable suppressedUnlessThereIsNoPrimary) { + if (maybePrimary == null) { + return suppressedUnlessThereIsNoPrimary; + } + if (suppressedUnlessThereIsNoPrimary != maybePrimary) { + maybePrimary.addSuppressed(suppressedUnlessThereIsNoPrimary); + } + return maybePrimary; } - private RejectedExecutionException createBackingExecutorShutdownException() { - return new RejectedExecutionException(format("The backing executor %s is shut down", backingExecutor)); + private static void rethrowAsUnchecked(@Nullable final Throwable t) throws RuntimeException, Error { + if (t instanceof RuntimeException) { + throw (RuntimeException) t; + } else if (t instanceof Error) { + throw (Error) t; + } else if (t != null) { + throw new RuntimeException(null, t); + } } @Override public String toString() { return "DefaultAsyncClientExecutor{" + "backingExecutor=" + backingExecutor + + ", scheduledTasks=" + scheduledTasks + ", closed=" + closed + '}'; } @NotThreadSafe - private class ScheduledCallbackCompletion implements Runnable { - private final SingleResultCallback callback; + private class ScheduledRejectableRunnable implements RejectableRunnable { + private final RejectableRunnable task; @Nullable private ScheduledFuture scheduledFuture; - ScheduledCallbackCompletion(final SingleResultCallback callback) { - this.callback = callback; + ScheduledRejectableRunnable(final RejectableRunnable task) { + this.task = task; } void onScheduled(final ScheduledFuture scheduledFuture) { @@ -168,23 +166,32 @@ void onScheduled(final ScheduledFuture scheduledFuture) { this.scheduledFuture = scheduledFuture; } - void reject(final RejectedExecutionException cause) { - if (scheduledCallbackCompletions.remove(this)) { + @Override + public void reject(final RejectedExecutionException cause) { + if (scheduledTasks.remove(this)) { try { if (scheduledFuture != null) { scheduledFuture.cancel(false); } } finally { - callback.completeExceptionally(cause); + task.reject(cause); } } } @Override public void run() { - if (scheduledCallbackCompletions.remove(this)) { - callback.complete(callback); + if (scheduledTasks.remove(this)) { + task.run(); } } + + @Override + public String toString() { + return "ScheduledRejectableRunnable{" + + "task=" + task + + ", scheduledFuture=" + scheduledFuture + + '}'; + } } } diff --git a/driver-core/src/main/com/mongodb/internal/thread/NoOpAsyncClientExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/NoOpAsyncClientExecutor.java index e2ab719e233..485ff7e490d 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/NoOpAsyncClientExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/NoOpAsyncClientExecutor.java @@ -16,7 +16,6 @@ package com.mongodb.internal.thread; import com.mongodb.annotations.ThreadSafe; -import com.mongodb.internal.async.SingleResultCallback; import java.time.Duration; @@ -30,14 +29,9 @@ final class NoOpAsyncClientExecutor implements AsyncClientExecutor { NoOpAsyncClientExecutor() { } - /** - * Must not be called with any {@link Duration} except {@linkplain Duration#isZero() zero}. - */ @Override - public void sleepAsync(final Duration duration, final SingleResultCallback callback) { - DefaultAsyncClientExecutor.sleepAsync(false, duration, callback, () -> { - throw fail(); - }); + public void schedule(final RejectableRunnable task, final Duration delay) { + fail(); } /** diff --git a/driver-core/src/main/com/mongodb/internal/thread/ThreadUtil.java b/driver-core/src/main/com/mongodb/internal/thread/ThreadUtil.java new file mode 100644 index 00000000000..53af09d94d1 --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/thread/ThreadUtil.java @@ -0,0 +1,67 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.internal.async.SingleResultCallback; +import com.mongodb.internal.thread.AsyncClientExecutor.RejectableRunnable; + +import java.time.Duration; + +import static com.mongodb.assertions.Assertions.assertFalse; +import static com.mongodb.internal.async.AsyncRunnable.beginAsync; +import static com.mongodb.internal.thread.InterruptionUtil.interruptAndCreateMongoInterruptedException; +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +/** + * This class is not part of the public API and may be removed or changed at any time. + */ +public final class ThreadUtil { + /** + * A convenient alternative to {@link Thread#sleep(long, int)}. + */ + public static void sleep(final Duration duration) { + try { + NANOSECONDS.sleep(duration.toNanos()); + } catch (InterruptedException e) { + throw interruptAndCreateMongoInterruptedException(null, e); + } + } + + /** + * The callback-based counterpart to {@link #sleep(Duration)}. + * + * @param duration A non-{@linkplain Duration#isNegative() negative} duration. + * If {@code duration} is {@linkplain Duration#isZero() zero}, + * the {@code callback} is {@linkplain SingleResultCallback#complete(SingleResultCallback) completed} + * by the {@link Thread} that invokes the method, and {@code clientExecutor} is not used. + */ + public static void sleepAsync( + final Duration duration, + final AsyncClientExecutor clientExecutor, + final SingleResultCallback callback) { + beginAsync().thenRun(c -> { + assertFalse(duration.isNegative()); + if (duration.isZero()) { + c.complete(c); + } else { + clientExecutor.schedule(RejectableRunnable.from(callback), duration); + } + }).finish(callback); + } + + private ThreadUtil() { + } +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java index 2771fbab2ad..efcb4b67dba 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/DefaultAsyncClientExecutorTest.java @@ -15,6 +15,7 @@ */ package com.mongodb.internal.thread; +import com.mongodb.internal.thread.AsyncClientExecutor.RejectableRunnable; import com.mongodb.internal.time.StartTime; import io.netty.channel.EventLoopGroup; import org.junit.jupiter.api.AfterEach; @@ -46,9 +47,10 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; class DefaultAsyncClientExecutorTest { - private static final long SLEEP_DURATION_MILLIS = 200; + private static final long SCHEDULE_DELAY_MILLIS = 200; private ExecutorService executorService; private ScheduledExecutorService scheduledExecutorService; @@ -70,53 +72,49 @@ void afterEach() { } @ParameterizedTest - @ValueSource(longs = {0, SLEEP_DURATION_MILLIS}) - void sleepAsync(final long durationMs) { - Duration duration = Duration.ofMillis(durationMs); + @ValueSource(longs = {0, SCHEDULE_DELAY_MILLIS}) + void schedule(final long delayMs) { + Duration delay = Duration.ofMillis(delayMs); try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { assertAll( - () -> assertSleepAsync(backedByExecutorService, duration), - () -> assertSleepAsync(backedByScheduledExecutorService, duration) + () -> assertSchedule(backedByExecutorService, delay), + () -> assertSchedule(backedByScheduledExecutorService, delay) ); } } - private static void assertSleepAsync( - final DefaultAsyncClientExecutor clientExecutor, final Duration duration) throws Exception { + private static void assertSchedule( + final DefaultAsyncClientExecutor clientExecutor, final Duration delay) throws Exception { StartTime startTime = StartTime.now(); CompletableFuture callbackDelayFuture = new CompletableFuture<>(); CompletableFuture callbackThreadFuture = new CompletableFuture<>(); - clientExecutor.sleepAsync(duration, (result, t) -> { + clientExecutor.schedule(RejectableRunnable.from((result, t) -> { if (t != null) { callbackDelayFuture.completeExceptionally(t); } else { callbackDelayFuture.complete(startTime.elapsed()); } callbackThreadFuture.complete(Thread.currentThread()); - }); - // if `duration` is zero, the futures are expected to be completed synchronously - long timeoutMs = duration.toMillis() * 2; + }), delay); + long timeoutMs = delay.isZero() ? SCHEDULE_DELAY_MILLIS : delay.toMillis() * 2; Duration actualCallbackDelay = callbackDelayFuture.get(timeoutMs, MILLISECONDS); Thread actualCallbackThread = callbackThreadFuture.get(timeoutMs, MILLISECONDS); - assertTrue(actualCallbackDelay.compareTo(duration) >= 0); - if (duration.isZero()) { - assertSame(Thread.currentThread(), actualCallbackThread); - } else { - assertTrue(actualCallbackDelay.compareTo(duration.multipliedBy(2)) < 0); - assertNotSame(Thread.currentThread(), actualCallbackThread); - } + assertTrue(actualCallbackDelay.compareTo(delay) >= 0); + Duration expectedMaxDelay = delay.isZero() ? Duration.ofMillis(SCHEDULE_DELAY_MILLIS) : delay.multipliedBy(2); + assertTrue(actualCallbackDelay.compareTo(expectedMaxDelay) < 0); + assertNotSame(Thread.currentThread(), actualCallbackThread); } @ParameterizedTest - @ValueSource(longs = {0, SLEEP_DURATION_MILLIS}) - void closeBeforeSleepAsync(final long durationMs) { - Duration duration = Duration.ofMillis(durationMs); + @ValueSource(longs = {0, SCHEDULE_DELAY_MILLIS}) + void closeBeforeSchedule(final long delayMs) { + Duration delay = Duration.ofMillis(delayMs); try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { assertAll( - () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByExecutorService, duration, backedByExecutorService::close), - () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByScheduledExecutorService, duration, backedByScheduledExecutorService::close) + () -> assertCloseOrBackingExecutorShutdownBeforeSchedule(backedByExecutorService, delay, backedByExecutorService::close), + () -> assertCloseOrBackingExecutorShutdownBeforeSchedule(backedByScheduledExecutorService, delay, backedByScheduledExecutorService::close) ); } } @@ -126,62 +124,62 @@ void closeBeforeSleepAsync(final long durationMs) { * forbit this scenario, but we still handle it. */ @Test - void backingExecutorShutdownBeforeSleepAsync() { - Duration duration = Duration.ofMillis(SLEEP_DURATION_MILLIS); + void backingExecutorShutdownBeforeSchedule() { + Duration delay = Duration.ofMillis(SCHEDULE_DELAY_MILLIS); try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { assertAll( - () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByExecutorService, duration, executorService::shutdownNow), - () -> assertCloseOrBackingExecutorShutdownBeforeSleepAsync(backedByScheduledExecutorService, duration, scheduledExecutorService::shutdownNow) + () -> assertCloseOrBackingExecutorShutdownBeforeSchedule(backedByExecutorService, delay, executorService::shutdownNow), + () -> assertCloseOrBackingExecutorShutdownBeforeSchedule(backedByScheduledExecutorService, delay, scheduledExecutorService::shutdownNow) ); } } - private static void assertCloseOrBackingExecutorShutdownBeforeSleepAsync( - final DefaultAsyncClientExecutor clientExecutor, final Duration duration, final Runnable doBeforeSleepAsync) throws Exception { - doBeforeSleepAsync.run(); + private static void assertCloseOrBackingExecutorShutdownBeforeSchedule( + final DefaultAsyncClientExecutor clientExecutor, final Duration delay, final Runnable doBeforeSchedule) throws Exception { + doBeforeSchedule.run(); AtomicInteger completionCount = new AtomicInteger(); CompletableFuture callbackFuture = new CompletableFuture<>(); - clientExecutor.sleepAsync(duration, (result, t) -> { + clientExecutor.schedule(RejectableRunnable.from((result, t) -> { completionCount.incrementAndGet(); if (t != null) { callbackFuture.completeExceptionally(t); } else { callbackFuture.complete(result); } - }); + }), delay); Throwable callbackException = assertThrows(CompletionException.class, () -> callbackFuture.getNow(null)).getCause(); assertInstanceOf(RejectedExecutionException.class, callbackException); - Thread.sleep(duration.toMillis() * 2); + Thread.sleep(delay.isZero() ? SCHEDULE_DELAY_MILLIS : delay.toMillis() * 2); assertEquals(1, completionCount.get()); } @Test - void closeWhileSleepingInSleepAsync() { - Duration duration = Duration.ofMillis(SLEEP_DURATION_MILLIS); + void closeWhileTaskIsWaitingToBeExecutedAfterSchedule() { + Duration delay = Duration.ofMillis(SCHEDULE_DELAY_MILLIS); try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { assertAll( - () -> assertCloseWhileSleepingInSleepAsync(backedByExecutorService, duration), - () -> assertCloseWhileSleepingInSleepAsync(backedByScheduledExecutorService, duration) + () -> assertCloseWhileTaskIsWaitingToBeExecutedAfterSchedule(backedByExecutorService, delay), + () -> assertCloseWhileTaskIsWaitingToBeExecutedAfterSchedule(backedByScheduledExecutorService, delay) ); } } - private static void assertCloseWhileSleepingInSleepAsync( - final DefaultAsyncClientExecutor clientExecutor, final Duration duration) throws Exception { + private static void assertCloseWhileTaskIsWaitingToBeExecutedAfterSchedule( + final DefaultAsyncClientExecutor clientExecutor, final Duration delay) throws Exception { AtomicInteger completionCount = new AtomicInteger(); CompletableFuture callbackFuture = new CompletableFuture<>(); - clientExecutor.sleepAsync(duration, (result, t) -> { + clientExecutor.schedule(RejectableRunnable.from((result, t) -> { completionCount.incrementAndGet(); if (t != null) { callbackFuture.completeExceptionally(t); } else { callbackFuture.complete(result); } - }); + }), delay); clientExecutor.close(); - long timeoutMs = duration.toMillis() * 2; + long timeoutMs = delay.toMillis() * 2; Throwable callbackException = assertThrows(ExecutionException.class, () -> callbackFuture.get(timeoutMs, MILLISECONDS)).getCause(); assertInstanceOf(RejectedExecutionException.class, callbackException); Thread.sleep(timeoutMs); @@ -189,40 +187,83 @@ private static void assertCloseWhileSleepingInSleepAsync( } @Test - void closeWhileCallbackIsBeingCompletedInSleepAsync() { - Duration duration = Duration.ofMillis(SLEEP_DURATION_MILLIS); + void closeWhileTaskIsBeingExecutedAfterSchedule() { + Duration delay = Duration.ofMillis(SCHEDULE_DELAY_MILLIS); try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { assertAll( - () -> assertCloseWhileCallbackIsBeingCompletedInSleepAsync(backedByExecutorService, duration), - () -> assertCloseWhileCallbackIsBeingCompletedInSleepAsync(backedByScheduledExecutorService, duration) + () -> assertCloseWhileTaskIsBeingExecutedAfterSchedule(backedByExecutorService, delay), + () -> assertCloseWhileTaskIsBeingExecutedAfterSchedule(backedByScheduledExecutorService, delay) ); } } - private static void assertCloseWhileCallbackIsBeingCompletedInSleepAsync( - final DefaultAsyncClientExecutor clientExecutor, final Duration duration) throws Exception { + private static void assertCloseWhileTaskIsBeingExecutedAfterSchedule( + final DefaultAsyncClientExecutor clientExecutor, final Duration delay) throws Exception { AtomicInteger completionCount = new AtomicInteger(); CompletableFuture callbackFuture = new CompletableFuture<>(); CompletableFuture closeFuture = new CompletableFuture<>(); - clientExecutor.sleepAsync(duration, (result, t) -> { + clientExecutor.schedule(RejectableRunnable.from((result, t) -> { completionCount.incrementAndGet(); if (t != null) { callbackFuture.completeExceptionally(t); } else { callbackFuture.complete(result); } - waitForCompletion(closeFuture, duration); - }); - waitForCompletion(callbackFuture, duration.multipliedBy(2)); + waitForCompletion(closeFuture, delay); + }), delay); + waitForCompletion(callbackFuture, delay.multipliedBy(2)); clientExecutor.close(); closeFuture.complete(null); - long timeoutMs = duration.toMillis() * 2; + long timeoutMs = delay.toMillis() * 2; assertDoesNotThrow(() -> callbackFuture.get(timeoutMs, MILLISECONDS)); Thread.sleep(timeoutMs); assertEquals(1, completionCount.get()); } + @Test + void closeWhileTaskIsWaitingToBeExecutedAfterScheduleExecutesAllTasksDespiteFailures() { + Duration delay = Duration.ofMillis(SCHEDULE_DELAY_MILLIS); + try (DefaultAsyncClientExecutor backedByExecutorService = new DefaultAsyncClientExecutor(executorService); + DefaultAsyncClientExecutor backedByScheduledExecutorService = new DefaultAsyncClientExecutor(scheduledExecutorService)) { + assertAll( + () -> assertCloseWhileTaskIsWaitingToBeExecutedAfterScheduleExecutesAllTasksDespiteFailures(backedByExecutorService, delay), + () -> assertCloseWhileTaskIsWaitingToBeExecutedAfterScheduleExecutesAllTasksDespiteFailures(backedByScheduledExecutorService, delay) + ); + } + } + + private static void assertCloseWhileTaskIsWaitingToBeExecutedAfterScheduleExecutesAllTasksDespiteFailures( + final DefaultAsyncClientExecutor clientExecutor, final Duration delay) { + AtomicInteger completionCount = new AtomicInteger(); + RuntimeException exception = new RuntimeException("must not prevent task execution caused by `close`"); + Error error = new Error("must not prevent task execution caused by `close`"); + clientExecutor.schedule(RejectableRunnable.from((result, t) -> { + completionCount.incrementAndGet(); + throw exception; + }), delay); + clientExecutor.schedule(RejectableRunnable.from((result, t) -> { + completionCount.incrementAndGet(); + throw exception; + }), delay); + clientExecutor.schedule(RejectableRunnable.from((result, t) -> { + completionCount.incrementAndGet(); + throw error; + }), delay); + Throwable actualThrowable = assertThrows(Throwable.class, clientExecutor::close); + // the order of task execution caused by `close` is indeterministic, so we handle all possibilities + if (actualThrowable instanceof RuntimeException) { + assertSame(exception, actualThrowable); + assertSame(error, actualThrowable.getSuppressed()[0]); + } else if (actualThrowable instanceof Error) { + assertSame(error, actualThrowable); + assertSame(exception, actualThrowable.getSuppressed()[0]); + } else { + fail(actualThrowable); + } + assertEquals(3, completionCount.get()); + } + private static void waitForCompletion(final Future future, final Duration duration) { try { future.get(duration.toNanos(), NANOSECONDS); diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/NoOpAsyncClientExecutorTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/NoOpAsyncClientExecutorTest.java deleted file mode 100644 index 064fecbe0b4..00000000000 --- a/driver-core/src/test/unit/com/mongodb/internal/thread/NoOpAsyncClientExecutorTest.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2008-present MongoDB, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.mongodb.internal.thread; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - -import java.time.Duration; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; - -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; - -class NoOpAsyncClientExecutorTest { - @Test - void noOpConstantIsOfTheRightType() { - assertInstanceOf(NoOpAsyncClientExecutor.class, AsyncClientExecutor.NO_OP); - } - - @ParameterizedTest - @ValueSource(longs = {0, 200}) - void sleepAsync(final long durationMs) { - Duration duration = Duration.ofMillis(durationMs); - assertSleepAsync(duration); - } - - private static void assertSleepAsync(final Duration duration) { - CompletableFuture callbackExceptionFuture = new CompletableFuture<>(); - CompletableFuture callbackThreadFuture = new CompletableFuture<>(); - try (AsyncClientExecutor noOpClientExecutor = AsyncClientExecutor.NO_OP) { - noOpClientExecutor.sleepAsync(duration, (result, t) -> { - if (t != null) { - callbackExceptionFuture.completeExceptionally(t); - } else { - callbackExceptionFuture.complete(result); - } - callbackThreadFuture.complete(Thread.currentThread()); - }); - } - if (duration.isZero()) { - assertDoesNotThrow(() -> callbackExceptionFuture.getNow(null)); - } else { - Throwable callbackException = assertThrows(CompletionException.class, () -> callbackExceptionFuture.getNow(null)).getCause(); - assertSame(AssertionError.class, callbackException.getClass()); - } - Thread actualCallbackThread = callbackThreadFuture.getNow(null); - assertSame(Thread.currentThread(), actualCallbackThread); - } -} diff --git a/driver-core/src/test/unit/com/mongodb/internal/thread/ThreadUtilTest.java b/driver-core/src/test/unit/com/mongodb/internal/thread/ThreadUtilTest.java new file mode 100644 index 00000000000..5178e4dfe07 --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/thread/ThreadUtilTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mongodb.internal.thread; + +import com.mongodb.internal.async.MutableValue; +import com.mongodb.internal.mockito.MongoMockito; +import com.mongodb.internal.thread.AsyncClientExecutor.RejectableRunnable; +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.doAnswer; + +class ThreadUtilTest { + @Test + void sleepAsyncCompletesInCallingThreadIfNoDelay() { + AsyncClientExecutor failingClientExecutor = MongoMockito.mock(AsyncClientExecutor.class); + Thread expectedThread = Thread.currentThread(); + // we use `AtomicReference` instead of `MutableValue` in case completion incorrectly happens in a different thread + AtomicReference actualThread = new AtomicReference<>(); + AtomicReference actualThrowable = new AtomicReference<>(); + ThreadUtil.sleepAsync(Duration.ZERO, failingClientExecutor, (result, t) -> { + actualThread.set(Thread.currentThread()); + actualThrowable.set(t); + }); + assertSame(expectedThread, actualThread.get()); + assertNull(actualThrowable.get()); + } + + @Test + void sleepAsyncSchedulesIfDelay() { + RejectedExecutionException expectedRejectionCauseFromClientExecutor = new RejectedExecutionException(); + Duration delay = Duration.ofNanos(1); + AsyncClientExecutor clientExecutor = MongoMockito.mock(AsyncClientExecutor.class, mock -> { + doAnswer(invocation -> { + RejectableRunnable task = invocation.getArgument(0); + task.reject(expectedRejectionCauseFromClientExecutor); + return null; + }).when(mock).schedule(any(), same(delay)); + }); + MutableValue completedByClientExecutor = new MutableValue<>(); + ThreadUtil.sleepAsync(delay, clientExecutor, (result, t) -> { + completedByClientExecutor.set(t); + }); + assertSame(expectedRejectionCauseFromClientExecutor, completedByClientExecutor.getNullable()); + } +} From 66182bc25316880df17609148ff9efd96436a42c Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 17 Aug 2026 15:48:04 -0600 Subject: [PATCH 13/14] Refactor `AsynchronousSocketChannelStreamFactoryFactory` such that its `getExecutor` returns the actual I/O executor --- .../AsynchronousSocketChannelStream.java | 31 ++++++---- ...synchronousSocketChannelStreamFactory.java | 24 ++++---- ...nousSocketChannelStreamFactoryFactory.java | 60 ++++++++++++------- .../connection/StreamFactoryHelper.java | 12 +--- ...yncSocketChannelStreamSpecification.groovy | 6 ++ ...elStreamFactoryFactorySpecification.groovy | 7 ++- 6 files changed, 80 insertions(+), 60 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStream.java b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStream.java index c60981c115e..f9939ae66d3 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStream.java +++ b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStream.java @@ -19,8 +19,10 @@ import com.mongodb.MongoSocketException; import com.mongodb.MongoSocketOpenException; import com.mongodb.ServerAddress; +import com.mongodb.annotations.ThreadSafe; import com.mongodb.connection.AsyncCompletionHandler; import com.mongodb.connection.SocketSettings; +import com.mongodb.internal.VisibleForTesting; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; @@ -38,6 +40,7 @@ import java.util.concurrent.atomic.AtomicReference; import static com.mongodb.assertions.Assertions.isTrue; +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; import static com.mongodb.internal.connection.ServerAddressHelper.getSocketAddresses; /** @@ -47,24 +50,24 @@ public final class AsynchronousSocketChannelStream extends AsynchronousChannelSt private final ServerAddress serverAddress; private final InetAddressResolver inetAddressResolver; private final SocketSettings settings; - @Nullable - private final AsynchronousChannelGroup group; + private final AsynchronousSocketChannelOpener channelOpener; + @VisibleForTesting(otherwise = PRIVATE) AsynchronousSocketChannelStream( final ServerAddress serverAddress, final InetAddressResolver inetAddressResolver, final SocketSettings settings, final PowerOfTwoBufferPool bufferProvider) { - this(serverAddress, inetAddressResolver, settings, bufferProvider, null); + this(serverAddress, inetAddressResolver, settings, bufferProvider, AsynchronousSocketChannel::open); } - public AsynchronousSocketChannelStream( + AsynchronousSocketChannelStream( final ServerAddress serverAddress, final InetAddressResolver inetAddressResolver, final SocketSettings settings, final PowerOfTwoBufferPool bufferProvider, - @Nullable final AsynchronousChannelGroup group) { + final AsynchronousSocketChannelOpener channelOpener) { super(serverAddress, settings, bufferProvider); this.serverAddress = serverAddress; this.inetAddressResolver = inetAddressResolver; this.settings = settings; - this.group = group; + this.channelOpener = channelOpener; } @Override @@ -89,10 +92,7 @@ private void initializeSocketChannel(final AsyncCompletionHandler handler, SocketAddress socketAddress = socketAddressQueue.poll(); try { - AsynchronousSocketChannel attemptConnectionChannel; - attemptConnectionChannel = group == null - ? AsynchronousSocketChannel.open() - : AsynchronousSocketChannel.open(group); + AsynchronousSocketChannel attemptConnectionChannel = channelOpener.open(); attemptConnectionChannel.setOption(StandardSocketOptions.TCP_NODELAY, true); attemptConnectionChannel.setOption(StandardSocketOptions.SO_KEEPALIVE, true); if (settings.getReceiveBufferSize() > 0) { @@ -207,4 +207,15 @@ public void close() throws IOException { channel.close(); } } + + /** + * An implementation must be thread-safe. + */ + @ThreadSafe + interface AsynchronousSocketChannelOpener { + /** + * See {@link AsynchronousSocketChannel#open(AsynchronousChannelGroup)}. + */ + AsynchronousSocketChannel open() throws IOException; + } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactory.java b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactory.java index 1ea15abe59d..7dfbe2097cc 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactory.java @@ -19,13 +19,15 @@ import com.mongodb.ServerAddress; import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; -import com.mongodb.lang.Nullable; +import com.mongodb.internal.VisibleForTesting; +import com.mongodb.internal.connection.AsynchronousSocketChannelStream.AsynchronousSocketChannelOpener; import com.mongodb.spi.dns.InetAddressResolver; -import java.nio.channels.AsynchronousChannelGroup; +import java.nio.channels.AsynchronousSocketChannel; import static com.mongodb.assertions.Assertions.assertFalse; import static com.mongodb.assertions.Assertions.notNull; +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; /** * Factory to create a Stream that's an AsynchronousSocketChannelStream. Throws an exception if SSL is enabled. @@ -34,34 +36,28 @@ public class AsynchronousSocketChannelStreamFactory implements StreamFactory { private final PowerOfTwoBufferPool bufferProvider = PowerOfTwoBufferPool.DEFAULT; private final SocketSettings settings; private final InetAddressResolver inetAddressResolver; - @Nullable - private final AsynchronousChannelGroup group; + private final AsynchronousSocketChannelOpener channelOpener; - /** - * Create a new factory with the default {@code BufferProvider} and {@code AsynchronousChannelGroup}. - * - * @param settings the settings for the connection to a MongoDB server - * @param sslSettings the settings for connecting via SSL - */ + @VisibleForTesting(otherwise = PRIVATE) public AsynchronousSocketChannelStreamFactory( final InetAddressResolver inetAddressResolver, final SocketSettings settings, final SslSettings sslSettings) { - this(inetAddressResolver, settings, sslSettings, null); + this(inetAddressResolver, settings, sslSettings, AsynchronousSocketChannel::open); } AsynchronousSocketChannelStreamFactory( final InetAddressResolver inetAddressResolver, final SocketSettings settings, - final SslSettings sslSettings, @Nullable final AsynchronousChannelGroup group) { + final SslSettings sslSettings, final AsynchronousSocketChannelOpener channelOpener) { assertFalse(sslSettings.isEnabled()); this.inetAddressResolver = inetAddressResolver; this.settings = notNull("settings", settings); - this.group = group; + this.channelOpener = channelOpener; } @Override public Stream create(final ServerAddress serverAddress) { return new AsynchronousSocketChannelStream( - serverAddress, inetAddressResolver, settings, bufferProvider, group); + serverAddress, inetAddressResolver, settings, bufferProvider, channelOpener); } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java index bde53275043..e63492e5dbe 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java +++ b/driver-core/src/main/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactory.java @@ -16,19 +16,25 @@ package com.mongodb.internal.connection; +import com.mongodb.MongoClientException; import com.mongodb.connection.AsyncTransportSettings; import com.mongodb.connection.SocketSettings; import com.mongodb.connection.SslSettings; +import com.mongodb.internal.VisibleForTesting; import com.mongodb.internal.thread.DaemonThreadFactory; import com.mongodb.internal.thread.MongoThreadPoolExecutor; import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; +import java.io.IOException; import java.nio.channels.AsynchronousChannelGroup; +import java.nio.channels.AsynchronousSocketChannel; import java.time.Duration; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; -import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.SynchronousQueue; + +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; /** * A {@code StreamFactoryFactory} implementation for AsynchronousSocketChannel-based streams. @@ -37,51 +43,59 @@ */ public final class AsynchronousSocketChannelStreamFactoryFactory implements StreamFactoryFactory { private final InetAddressResolver inetAddressResolver; - @Nullable private final AsynchronousChannelGroup group; - private final MongoThreadPoolExecutor ownedExecutorBackingClientExecutor; + private final ExecutorService ownedExecutorService; - public AsynchronousSocketChannelStreamFactoryFactory(final InetAddressResolver inetAddressResolver) { + @VisibleForTesting(otherwise = PRIVATE) + AsynchronousSocketChannelStreamFactoryFactory(final InetAddressResolver inetAddressResolver) { this(inetAddressResolver, null); } + /** + * @param applicationSuppliedOwnedExecutorService Owned {@link ExecutorService} from {@link AsyncTransportSettings#getExecutorService()}. + */ AsynchronousSocketChannelStreamFactoryFactory( final InetAddressResolver inetAddressResolver, - @Nullable final AsynchronousChannelGroup group) { + @Nullable final ExecutorService applicationSuppliedOwnedExecutorService) { this.inetAddressResolver = inetAddressResolver; - this.group = group; - int availableProcessors = Runtime.getRuntime().availableProcessors(); - ownedExecutorBackingClientExecutor = new MongoThreadPoolExecutor( - availableProcessors, availableProcessors, Duration.ofMinutes(5), - new LinkedBlockingQueue<>(), new DaemonThreadFactory("ClientExecutor")); - ownedExecutorBackingClientExecutor.allowCoreThreadTimeOut(true); + try { + if (applicationSuppliedOwnedExecutorService == null) { + // We try to create a group similarly to how + // the system-wide default `AsynchronousChannelGroup` is created. + // This means: + // - creating the executor similarly to `Executors.newCachedThreadPool`; + // - creating the group via `withCachedThreadPool`; + // - requesting the implementation specific default by passing negative `initialSize`. + ownedExecutorService = new MongoThreadPoolExecutor( + 0, Integer.MAX_VALUE, Duration.ofSeconds(60), new SynchronousQueue<>(), new DaemonThreadFactory("IOExecutor")); + group = AsynchronousChannelGroup.withCachedThreadPool(ownedExecutorService, -1); + } else { + ownedExecutorService = applicationSuppliedOwnedExecutorService; + group = AsynchronousChannelGroup.withThreadPool(ownedExecutorService); + } + } catch (IOException e) { + throw new MongoClientException("Unable to create an asynchronous channel group", e); + } } @Override public StreamFactory create(final SocketSettings socketSettings, final SslSettings sslSettings) { return new AsynchronousSocketChannelStreamFactory( - inetAddressResolver, socketSettings, sslSettings, group); + inetAddressResolver, socketSettings, sslSettings, () -> AsynchronousSocketChannel.open(group)); } /** - * @return VAKOTODO create ticket, leave a TODO - * A dedicated {@link MongoThreadPoolExecutor}, which is suboptimal. To make things right, this should be - * The {@link ExecutorService} used by the {@link StreamFactory} created via {@link #create(SocketSettings, SslSettings)}. + * @return The {@link ExecutorService} used by the {@link StreamFactory} created via {@link #create(SocketSettings, SslSettings)}. * It may be provided by an application via {@link AsyncTransportSettings#getExecutorService()}. */ @Override public Executor getExecutor() { - return ownedExecutorBackingClientExecutor; + return ownedExecutorService; } @Override public void close() { - try { - if (group != null) { - group.shutdown(); - } - } finally { - ownedExecutorBackingClientExecutor.shutdown(); - } + // termination of the `group` results in the orderly shutdown of the `ownedExecutorService` + group.shutdown(); } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java index b781bf9018b..b8b14e317ef 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java +++ b/driver-core/src/main/com/mongodb/internal/connection/StreamFactoryHelper.java @@ -27,8 +27,6 @@ import com.mongodb.lang.Nullable; import com.mongodb.spi.dns.InetAddressResolver; -import java.io.IOException; -import java.nio.channels.AsynchronousChannelGroup; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; @@ -79,15 +77,7 @@ public static StreamFactoryFactory getAsyncStreamFactoryFactory(final MongoClien if (settings.getSslSettings().isEnabled()) { return new TlsChannelStreamFactoryFactory(inetAddressResolver, executorService); } - AsynchronousChannelGroup group = null; - if (executorService != null) { - try { - group = AsynchronousChannelGroup.withThreadPool(executorService); - } catch (IOException e) { - throw new MongoClientException("Unable to create an asynchronous channel group", e); - } - } - return new AsynchronousSocketChannelStreamFactoryFactory(inetAddressResolver, group); + return new AsynchronousSocketChannelStreamFactoryFactory(inetAddressResolver, executorService); } else if (transportSettings instanceof NettyTransportSettings) { return getNettyStreamFactoryFactory(inetAddressResolver, (NettyTransportSettings) transportSettings); } else { diff --git a/driver-core/src/test/functional/com/mongodb/internal/connection/AsyncSocketChannelStreamSpecification.groovy b/driver-core/src/test/functional/com/mongodb/internal/connection/AsyncSocketChannelStreamSpecification.groovy index 2660693eccf..fa373e2e740 100644 --- a/driver-core/src/test/functional/com/mongodb/internal/connection/AsyncSocketChannelStreamSpecification.groovy +++ b/driver-core/src/test/functional/com/mongodb/internal/connection/AsyncSocketChannelStreamSpecification.groovy @@ -44,6 +44,9 @@ class AsyncSocketChannelStreamSpecification extends Specification { then: !stream.isClosed() + + cleanup: + factoryFactory.close() } @Slow @@ -70,6 +73,9 @@ class AsyncSocketChannelStreamSpecification extends Specification { then: thrown(MongoSocketOpenException) + + cleanup: + factoryFactory.close() } @IgnoreIf({ getSslSettings().isEnabled() }) diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactorySpecification.groovy b/driver-core/src/test/unit/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactorySpecification.groovy index 245c6c87a5a..c41c5fc8aa1 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactorySpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/AsynchronousSocketChannelStreamFactoryFactorySpecification.groovy @@ -27,8 +27,8 @@ class AsynchronousSocketChannelStreamFactoryFactorySpecification extends Specifi @Unroll def 'should create the expected #description AsynchronousSocketChannelStream'() { given: - def factory = new AsynchronousSocketChannelStreamFactoryFactory(new DefaultInetAddressResolver()) - .create(socketSettings, sslSettings) + def factoryFactory = new AsynchronousSocketChannelStreamFactoryFactory(new DefaultInetAddressResolver()) + def factory = factoryFactory.create(socketSettings, sslSettings) when: AsynchronousSocketChannelStream stream = factory.create(serverAddress) as AsynchronousSocketChannelStream @@ -36,6 +36,9 @@ class AsynchronousSocketChannelStreamFactoryFactorySpecification extends Specifi then: stream.getSettings() == socketSettings stream.getAddress() == serverAddress + + cleanup: + factoryFactory.close() } SocketSettings socketSettings = SocketSettings.builder().build() From bbe71e8193525d5de770a9acec46d723070c9bf9 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 17 Aug 2026 15:49:40 -0600 Subject: [PATCH 14/14] Make `CommonExecutor` package-access --- .../src/main/com/mongodb/internal/thread/CommonExecutor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java index 16dbac39d72..a8cb1569b29 100644 --- a/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java +++ b/driver-core/src/main/com/mongodb/internal/thread/CommonExecutor.java @@ -45,8 +45,8 @@ * * @see AsyncClientExecutor */ -// VAKOTODO decide what to do with https://jira.mongodb.org/browse/JAVA-6279. -public final class CommonExecutor { +// TODO-BACKPRESSURE Valentin decide what to do with https://jira.mongodb.org/browse/JAVA-6279. +final class CommonExecutor { private static final Logger LOGGER = Loggers.getLogger("client"); private static final CommonExecutor INSTANCE = new CommonExecutor();