From 0f31b05e566bb32439c446242b1efec0de01972b Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 25 Sep 2023 18:31:33 -0600 Subject: [PATCH 1/5] Handle interrupts: synchronous `KeyManagementService` uses Socket IO (open, read, write), which is interruptible in a virtual thread JAVA-5139 --- .../com/mongodb/client/internal/Crypt.java | 30 ++++++++++++------- .../client/internal/KeyManagementService.java | 22 ++++++++++++-- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java index 3235e286f7f..f12228ae56c 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java +++ b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java @@ -42,12 +42,14 @@ import java.io.InputStream; import java.nio.ByteBuffer; import java.util.Map; +import java.util.function.Function; import java.util.function.Supplier; import static com.mongodb.assertions.Assertions.assertNotNull; import static com.mongodb.assertions.Assertions.notNull; import static com.mongodb.crypt.capi.MongoCryptContext.State; import static com.mongodb.internal.client.vault.EncryptOptionsHelper.asMongoExplicitEncryptOptions; +import static com.mongodb.internal.thread.InterruptionUtil.translateInterruptedException; /** *

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

@@ -141,7 +143,7 @@ RawBsonDocument encrypt(final String databaseName, final RawBsonDocument command try (MongoCryptContext encryptionContext = mongoCrypt.createEncryptionContext(databaseName, command)) { return executeStateMachine(encryptionContext, databaseName); } catch (MongoCryptException e) { - throw wrapInClientException(e); + throw wrapInMongoException(e); } } @@ -156,7 +158,7 @@ RawBsonDocument decrypt(final RawBsonDocument commandResponse) { try (MongoCryptContext decryptionContext = mongoCrypt.createDecryptionContext(commandResponse)) { return executeStateMachine(decryptionContext, null); } catch (MongoCryptException e) { - throw wrapInClientException(e); + throw wrapInMongoException(e); } } @@ -179,7 +181,7 @@ BsonDocument createDataKey(final String kmsProvider, final DataKeyOptions option .build())) { return executeStateMachine(dataKeyCreationContext, null); } catch (MongoCryptException e) { - throw wrapInClientException(e); + throw wrapInMongoException(e); } } @@ -198,7 +200,7 @@ BsonBinary encryptExplicitly(final BsonValue value, final EncryptOptions options new BsonDocument("v", value), asMongoExplicitEncryptOptions(options))) { return executeStateMachine(encryptionContext, null).getBinary("v"); } catch (MongoCryptException e) { - throw wrapInClientException(e); + throw wrapInMongoException(e); } } @@ -218,7 +220,7 @@ BsonDocument encryptExpression(final BsonDocument expression, final EncryptOptio new BsonDocument("v", expression), asMongoExplicitEncryptOptions(options))) { return executeStateMachine(encryptionContext, null).getDocument("v"); } catch (MongoCryptException e) { - throw wrapInClientException(e); + throw wrapInMongoException(e); } } @@ -233,7 +235,7 @@ BsonValue decryptExplicitly(final BsonBinary value) { try (MongoCryptContext decryptionContext = mongoCrypt.createExplicitDecryptionContext(new BsonDocument("v", value))) { return assertNotNull(executeStateMachine(decryptionContext, null).get("v")); } catch (MongoCryptException e) { - throw wrapInClientException(e); + throw wrapInMongoException(e); } } @@ -256,7 +258,7 @@ BsonDocument rewrapManyDataKey(final BsonDocument filter, final RewrapManyDataKe return executeStateMachine(rewrapManyDatakeyContext, null); } } catch (MongoCryptException e) { - throw wrapInClientException(e); + throw wrapInMongoException(e); } } @@ -324,7 +326,7 @@ private void mark(final MongoCryptContext cryptContext, final String databaseNam cryptContext.addMongoOperationResult(markedCommand); cryptContext.completeMongoOperation(); } catch (Throwable t) { - throw wrapInClientException(t); + throw wrapInMongoException(t); } } @@ -348,7 +350,9 @@ private void decryptKeys(final MongoCryptContext cryptContext) { } cryptContext.completeKeyDecryptors(); } catch (Throwable t) { - throw wrapInClientException(t); + throw translateInterruptedException(t, "Interrupted while reading") + .map(Function.identity()) + .orElseGet(() -> wrapInMongoException(t)); } } @@ -366,7 +370,11 @@ private void decryptKey(final MongoKeyDecryptor keyDecryptor) throws IOException } } - private MongoClientException wrapInClientException(final Throwable t) { - return new MongoClientException("Exception in encryption library: " + t.getMessage(), t); + private MongoException wrapInMongoException(final Throwable t) { + if (t instanceof MongoException) { + return (MongoException) t; + } else { + return new MongoClientException("Exception in encryption library: " + t.getMessage(), t); + } } } diff --git a/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java b/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java index c6c04eec0c3..d7ccf3a8646 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java +++ b/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java @@ -16,6 +16,7 @@ package com.mongodb.client.internal; +import com.mongodb.MongoInterruptedException; import com.mongodb.ServerAddress; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; @@ -34,8 +35,10 @@ import java.net.Socket; import java.nio.ByteBuffer; import java.util.Map; +import java.util.Optional; import static com.mongodb.assertions.Assertions.notNull; +import static com.mongodb.internal.thread.InterruptionUtil.translateInterruptedException; class KeyManagementService { private static final Logger LOGGER = Loggers.getLogger("client"); @@ -63,7 +66,7 @@ public InputStream stream(final String kmsProvider, final String host, final Byt socket.connect(new InetSocketAddress(InetAddress.getByName(serverAddress.getHost()), serverAddress.getPort()), timeoutMillis); } catch (IOException e) { closeSocket(socket); - throw e; + throw handleInterruptAndThrow(e, "Interrupted while connecting"); } try { @@ -75,7 +78,7 @@ public InputStream stream(final String kmsProvider, final String host, final Byt outputStream.write(bytes); } catch (IOException e) { closeSocket(socket); - throw e; + throw handleInterruptAndThrow(e, "Interrupted while writing"); } try { @@ -98,8 +101,21 @@ private void enableHostNameVerification(final SSLSocket socket) { private void closeSocket(final Socket socket) { try { socket.close(); - } catch (IOException e) { + } catch (IOException | RuntimeException e) { // ignore } } + + /** + * @return Never. + */ + private static RuntimeException handleInterruptAndThrow(final IOException e, final String message) throws IOException, + MongoInterruptedException { + Optional interruptedException = translateInterruptedException(e, message); + if (interruptedException.isPresent()) { + throw interruptedException.get(); + } else { + throw e; + } + } } From dbde767f888ac630f8cea20a7ff6bbb149b5d4a6 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 26 Sep 2023 08:53:42 -0600 Subject: [PATCH 2/5] Rename the new method JAVA-5139 --- .../com/mongodb/client/internal/KeyManagementService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java b/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java index d7ccf3a8646..e5902f617da 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java +++ b/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java @@ -66,7 +66,7 @@ public InputStream stream(final String kmsProvider, final String host, final Byt socket.connect(new InetSocketAddress(InetAddress.getByName(serverAddress.getHost()), serverAddress.getPort()), timeoutMillis); } catch (IOException e) { closeSocket(socket); - throw handleInterruptAndThrow(e, "Interrupted while connecting"); + throw translateInterruptedExceptionAndThrow(e, "Interrupted while connecting"); } try { @@ -78,7 +78,7 @@ public InputStream stream(final String kmsProvider, final String host, final Byt outputStream.write(bytes); } catch (IOException e) { closeSocket(socket); - throw handleInterruptAndThrow(e, "Interrupted while writing"); + throw translateInterruptedExceptionAndThrow(e, "Interrupted while writing"); } try { @@ -109,7 +109,7 @@ private void closeSocket(final Socket socket) { /** * @return Never. */ - private static RuntimeException handleInterruptAndThrow(final IOException e, final String message) throws IOException, + private static RuntimeException translateInterruptedExceptionAndThrow(final IOException e, final String message) throws IOException, MongoInterruptedException { Optional interruptedException = translateInterruptedException(e, message); if (interruptedException.isPresent()) { From 1241d27305e4706d253cdd71e104ee81e699ef41 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 26 Sep 2023 15:46:29 -0600 Subject: [PATCH 3/5] Simplify JAVA-5139 --- .../com/mongodb/client/internal/Crypt.java | 2 +- .../client/internal/KeyManagementService.java | 20 ++----------------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java index f12228ae56c..e8a104db064 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java +++ b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java @@ -350,7 +350,7 @@ private void decryptKeys(final MongoCryptContext cryptContext) { } cryptContext.completeKeyDecryptors(); } catch (Throwable t) { - throw translateInterruptedException(t, "Interrupted while reading") + throw translateInterruptedException(t, "Interrupted while doing IO") .map(Function.identity()) .orElseGet(() -> wrapInMongoException(t)); } diff --git a/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java b/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java index e5902f617da..7ae6f106ed5 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java +++ b/driver-sync/src/main/com/mongodb/client/internal/KeyManagementService.java @@ -16,7 +16,6 @@ package com.mongodb.client.internal; -import com.mongodb.MongoInterruptedException; import com.mongodb.ServerAddress; import com.mongodb.internal.diagnostics.logging.Logger; import com.mongodb.internal.diagnostics.logging.Loggers; @@ -35,10 +34,8 @@ import java.net.Socket; import java.nio.ByteBuffer; import java.util.Map; -import java.util.Optional; import static com.mongodb.assertions.Assertions.notNull; -import static com.mongodb.internal.thread.InterruptionUtil.translateInterruptedException; class KeyManagementService { private static final Logger LOGGER = Loggers.getLogger("client"); @@ -66,7 +63,7 @@ public InputStream stream(final String kmsProvider, final String host, final Byt socket.connect(new InetSocketAddress(InetAddress.getByName(serverAddress.getHost()), serverAddress.getPort()), timeoutMillis); } catch (IOException e) { closeSocket(socket); - throw translateInterruptedExceptionAndThrow(e, "Interrupted while connecting"); + throw e; } try { @@ -78,7 +75,7 @@ public InputStream stream(final String kmsProvider, final String host, final Byt outputStream.write(bytes); } catch (IOException e) { closeSocket(socket); - throw translateInterruptedExceptionAndThrow(e, "Interrupted while writing"); + throw e; } try { @@ -105,17 +102,4 @@ private void closeSocket(final Socket socket) { // ignore } } - - /** - * @return Never. - */ - private static RuntimeException translateInterruptedExceptionAndThrow(final IOException e, final String message) throws IOException, - MongoInterruptedException { - Optional interruptedException = translateInterruptedException(e, message); - if (interruptedException.isPresent()) { - throw interruptedException.get(); - } else { - throw e; - } - } } From 424e1d5b3a54f5b06fd088584dc9ddf488db9801 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Thu, 28 Sep 2023 20:25:19 -0600 Subject: [PATCH 4/5] Implement the change similar to the one proposed in a different PR See https://github.com/mongodb/mongo-java-driver/pull/1203#discussion_r1340826971 JAVA-5139 --- driver-sync/src/main/com/mongodb/client/internal/Crypt.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java index e8a104db064..c6fcbdd1688 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java +++ b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java @@ -351,8 +351,7 @@ private void decryptKeys(final MongoCryptContext cryptContext) { cryptContext.completeKeyDecryptors(); } catch (Throwable t) { throw translateInterruptedException(t, "Interrupted while doing IO") - .map(Function.identity()) - .orElseGet(() -> wrapInMongoException(t)); + .orElseThrow(() -> wrapInMongoException(t)); } } From 3f975b3e9bf6a6b6e8d6f7b484003e248659343a Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Thu, 28 Sep 2023 21:23:18 -0600 Subject: [PATCH 5/5] Fix static checks JAVA-5139 --- driver-sync/src/main/com/mongodb/client/internal/Crypt.java | 1 - 1 file changed, 1 deletion(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java index c6fcbdd1688..792061d7748 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java +++ b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java @@ -42,7 +42,6 @@ import java.io.InputStream; import java.nio.ByteBuffer; import java.util.Map; -import java.util.function.Function; import java.util.function.Supplier; import static com.mongodb.assertions.Assertions.assertNotNull;