From b7be8c856f8e500dcc10ca39757c8dccf33769c3 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 8 Jul 2024 18:00:46 -0600 Subject: [PATCH 1/3] Add `attemptJava5516` - a non-deterministic reproducer JAVA-5516 --- ...AsyncCommandBatchCursorFunctionalTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java b/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java index 3b8addf6596..147d0e268bd 100644 --- a/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java +++ b/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java @@ -45,6 +45,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -299,6 +300,24 @@ void testLimitWithGetMore() { assertTrue(cursor.isClosed()); } + @Test + void attemptJava5516() { + BsonDocument commandResult = executeFindCommand(5, 2); + cursor = new AsyncCommandBatchCursor<>(commandResult, 2, 0, DOCUMENT_DECODER, + null, connectionSource, connection); + assertNotNull(cursorNext()); + // Calling `close` twice is the key to reproducing. + // It does not matter whether we call `close` twice from the same thread or not. + ForkJoinPool.commonPool().execute(() -> cursor.close()); + ForkJoinPool.commonPool().execute(() -> cursor.close()); + try { + assertNotNull(cursorNext()); + assertNotNull(cursorNext()); + } catch (IllegalStateException e) { + // one of the expected outcomes, because we call `cursorNext` concurrently with `close` + } + } + @Test @DisplayName("test limit with large documents") void testLimitWithLargeDocuments() { From 3f6bca4b0dc21866a4c16f6d6e24038c9317acbb Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 8 Jul 2024 18:01:28 -0600 Subject: [PATCH 2/3] Fix JAVA-5516 --- .../com/mongodb/internal/operation/CursorResourceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/internal/operation/CursorResourceManager.java b/driver-core/src/main/com/mongodb/internal/operation/CursorResourceManager.java index cb2e5c58e84..7aeaad49118 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/CursorResourceManager.java +++ b/driver-core/src/main/com/mongodb/internal/operation/CursorResourceManager.java @@ -182,7 +182,7 @@ void endOperation() { void close() { boolean doClose = withLock(lock, () -> { State localState = state; - if (localState == State.OPERATION_IN_PROGRESS) { + if (localState.inProgress()) { state = State.CLOSE_PENDING; } else if (localState != State.CLOSED) { state = State.CLOSED; From 510548421e663d391a89557a9940afeacba2dc9c Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 9 Jul 2024 11:20:56 -0600 Subject: [PATCH 3/3] Introduce a simple deterministic reproducer JAVA-5516 --- ...AsyncCommandBatchCursorFunctionalTest.java | 19 ------ .../operation/CursorResourceManagerTest.java | 59 +++++++++++++++++++ 2 files changed, 59 insertions(+), 19 deletions(-) create mode 100644 driver-core/src/test/unit/com/mongodb/internal/operation/CursorResourceManagerTest.java diff --git a/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java b/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java index 147d0e268bd..3b8addf6596 100644 --- a/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java +++ b/driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorFunctionalTest.java @@ -45,7 +45,6 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -300,24 +299,6 @@ void testLimitWithGetMore() { assertTrue(cursor.isClosed()); } - @Test - void attemptJava5516() { - BsonDocument commandResult = executeFindCommand(5, 2); - cursor = new AsyncCommandBatchCursor<>(commandResult, 2, 0, DOCUMENT_DECODER, - null, connectionSource, connection); - assertNotNull(cursorNext()); - // Calling `close` twice is the key to reproducing. - // It does not matter whether we call `close` twice from the same thread or not. - ForkJoinPool.commonPool().execute(() -> cursor.close()); - ForkJoinPool.commonPool().execute(() -> cursor.close()); - try { - assertNotNull(cursorNext()); - assertNotNull(cursorNext()); - } catch (IllegalStateException e) { - // one of the expected outcomes, because we call `cursorNext` concurrently with `close` - } - } - @Test @DisplayName("test limit with large documents") void testLimitWithLargeDocuments() { diff --git a/driver-core/src/test/unit/com/mongodb/internal/operation/CursorResourceManagerTest.java b/driver-core/src/test/unit/com/mongodb/internal/operation/CursorResourceManagerTest.java new file mode 100644 index 00000000000..15a8bd972f1 --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/operation/CursorResourceManagerTest.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.operation; + +import com.mongodb.MongoNamespace; +import com.mongodb.ServerCursor; +import com.mongodb.internal.binding.AsyncConnectionSource; +import com.mongodb.internal.binding.ReferenceCounted; +import com.mongodb.internal.connection.Connection; +import com.mongodb.internal.mockito.MongoMockito; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.mockito.Mockito.when; + +final class CursorResourceManagerTest { + @Test + void doubleCloseExecutedConcurrentlyWithOperationBeingInProgressShouldNotFail() { + CursorResourceManager cursorResourceManager = new CursorResourceManager( + new MongoNamespace("db", "coll"), + MongoMockito.mock(AsyncConnectionSource.class, mock -> { + when(mock.retain()).thenReturn(mock); + when(mock.release()).thenReturn(1); + }), + null, + MongoMockito.mock(ServerCursor.class)) { + @Override + void markAsPinned(final ReferenceCounted connectionToPin, final Connection.PinningMode pinningMode) { + } + + @Override + void doClose() { + } + }; + cursorResourceManager.tryStartOperation(); + try { + assertDoesNotThrow(() -> { + cursorResourceManager.close(); + cursorResourceManager.close(); + cursorResourceManager.setServerCursor(null); + }); + } finally { + cursorResourceManager.endOperation(); + } + } +}