Skip to content

Commit 95fcc33

Browse files
jyeminjyemin
committed
Replace synchronized with ReentrantLock
In order to get the benefit for structured concurrency, the driver must use Lock#lockInterruptibly rather than simply Lock#lock. JAVA-4642
1 parent afb5d0a commit 95fcc33

File tree

7 files changed

+166
-32
lines changed

7 files changed

+166
-32
lines changed

‎driver-core/src/main/com/mongodb/KerberosSubjectProvider.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@
2626
import javax.security.auth.kerberos.KerberosTicket;
2727
import javax.security.auth.login.LoginContext;
2828
import javax.security.auth.login.LoginException;
29+
import java.util.concurrent.locks.ReentrantLock;
2930

3031
import static com.mongodb.assertions.Assertions.notNull;
32+
import static com.mongodb.internal.Locks.checkedSupplyWithLock;
3133
import static java.lang.String.format;
3234
import static java.util.concurrent.TimeUnit.MILLISECONDS;
3335
import static java.util.concurrent.TimeUnit.MINUTES;
@@ -54,6 +56,7 @@ public class KerberosSubjectProvider implements SubjectProvider {
5456
private static final Logger LOGGER = Loggers.getLogger("authenticator");
5557
private static final String TGT_PREFIX = "krbtgt/";
5658

59+
private final ReentrantLock lock = new ReentrantLock();
5760
private String loginContextName;
5861
private String fallbackLoginContextName;
5962
private Subject subject;
@@ -87,11 +90,13 @@ private KerberosSubjectProvider(final String loginContextName, @Nullable final S
8790
* @throws LoginException any exception resulting from a call to {@link LoginContext#login()}
8891
*/
8992
@NonNull
90-
public synchronized Subject getSubject() throws LoginException {
91-
if (subject == null || needNewSubject(subject)) {
92-
subject = createNewSubject();
93-
}
94-
return subject;
93+
public Subject getSubject() throws LoginException {
94+
return checkedSupplyWithLock(lock, () -> {
95+
if (subject == null || needNewSubject(subject)) {
96+
subject = createNewSubject();
97+
}
98+
return subject;
99+
});
95100
}
96101

97102
private Subject createNewSubject() throws LoginException {

‎driver-core/src/main/com/mongodb/internal/CheckedSupplier.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright 2008-present MongoDB, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.mongodb.internal;
18+
19+
/**
20+
* This class is not part of the public API and may be removed or changed at any time.
21+
*/
22+
@FunctionalInterface
23+
public interface CheckedSupplier<T, E extends Exception> {
24+
25+
/**
26+
* Gets a result.
27+
*
28+
* @return a result
29+
* @throws E the checked exception to throw
30+
*/
31+
T get() throws E;
32+
}

‎driver-core/src/main/com/mongodb/internal/Locks.java

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright 2008-present MongoDB, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.mongodb.internal;
18+
19+
import com.mongodb.MongoInterruptedException;
20+
21+
import java.util.concurrent.locks.Lock;
22+
import java.util.function.Supplier;
23+
24+
/**
25+
* This class is not part of the public API and may be removed or changed at any time.
26+
*/
27+
public final class Locks {
28+
public static void runWithLock(final Lock lock, final Runnable action) {
29+
try {
30+
lock.lockInterruptibly();
31+
try {
32+
action.run();
33+
} finally {
34+
lock.unlock();
35+
}
36+
} catch (InterruptedException e) {
37+
Thread.currentThread().interrupt();
38+
throw new MongoInterruptedException("Interrupted waiting for lock", e);
39+
}
40+
}
41+
42+
public static <V, E extends Exception> V checkedSupplyWithLock(final Lock lock, final CheckedSupplier<V, E> supplier) throws E {
43+
try {
44+
lock.lockInterruptibly();
45+
try {
46+
return supplier.get();
47+
} finally {
48+
lock.unlock();
49+
}
50+
} catch (InterruptedException e) {
51+
Thread.currentThread().interrupt();
52+
throw new MongoInterruptedException("Interrupted waiting for lock", e);
53+
}
54+
}
55+
56+
public static <V> V supplyWithLock(final Lock lock, final Supplier<V> supplier) {
57+
try {
58+
lock.lockInterruptibly();
59+
try {
60+
return supplier.get();
61+
} finally {
62+
lock.unlock();
63+
}
64+
} catch (InterruptedException e) {
65+
Thread.currentThread().interrupt();
66+
throw new MongoInterruptedException("Interrupted waiting for lock", e);
67+
}
68+
}
69+
70+
private Locks() {
71+
}
72+
}

‎driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,15 @@
4848
import java.util.concurrent.CountDownLatch;
4949
import java.util.concurrent.ThreadLocalRandom;
5050
import java.util.concurrent.atomic.AtomicReference;
51+
import java.util.concurrent.locks.ReentrantLock;
5152
import java.util.function.Function;
5253

5354
import static com.mongodb.assertions.Assertions.isTrue;
5455
import static com.mongodb.assertions.Assertions.notNull;
5556
import static com.mongodb.connection.ServerDescription.MAX_DRIVER_WIRE_VERSION;
5657
import static com.mongodb.connection.ServerDescription.MIN_DRIVER_SERVER_VERSION;
5758
import static com.mongodb.connection.ServerDescription.MIN_DRIVER_WIRE_VERSION;
59+
import static com.mongodb.internal.Locks.runWithLock;
5860
import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE;
5961
import static com.mongodb.internal.connection.EventHelper.wouldDescriptionsGenerateEquivalentEvents;
6062
import static com.mongodb.internal.event.EventListenerHelper.singleClusterListener;
@@ -68,6 +70,7 @@ abstract class BaseCluster implements Cluster {
6870

6971
private static final Logger LOGGER = Loggers.getLogger("cluster");
7072

73+
private final ReentrantLock lock = new ReentrantLock();
7174
private final AtomicReference<CountDownLatch> phase = new AtomicReference<CountDownLatch>(new CountDownLatch(1));
7275
private final ClusterableServerFactory serverFactory;
7376
private final ClusterId clusterId;
@@ -268,8 +271,8 @@ public ClusterDescription getCurrentDescription() {
268271
}
269272

270273
@Override
271-
public synchronized void withLock(final Runnable action) {
272-
action.run();
274+
public void withLock(final Runnable action) {
275+
runWithLock(lock, action);
273276
}
274277

275278
private void updatePhase() {

‎driver-core/src/main/com/mongodb/internal/connection/ClusterClock.java

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,29 +19,37 @@
1919
import org.bson.BsonDocument;
2020
import org.bson.BsonTimestamp;
2121

22+
import java.util.concurrent.locks.ReentrantLock;
23+
24+
import static com.mongodb.internal.Locks.runWithLock;
25+
import static com.mongodb.internal.Locks.supplyWithLock;
26+
2227
public class ClusterClock {
2328
private static final String CLUSTER_TIME_KEY = "clusterTime";
29+
private final ReentrantLock lock = new ReentrantLock();
2430
private BsonDocument clusterTime;
2531

26-
public synchronized BsonDocument getCurrent() {
27-
return clusterTime;
32+
public BsonDocument getCurrent() {
33+
return supplyWithLock(lock, () -> clusterTime);
2834
}
2935

30-
public synchronized BsonTimestamp getClusterTime() {
31-
return clusterTime != null ? clusterTime.getTimestamp(CLUSTER_TIME_KEY) : null;
36+
public BsonTimestamp getClusterTime() {
37+
return supplyWithLock(lock, () -> clusterTime != null ? clusterTime.getTimestamp(CLUSTER_TIME_KEY) : null);
3238
}
3339

34-
public synchronized void advance(final BsonDocument other) {
35-
this.clusterTime = greaterOf(other);
40+
public void advance(final BsonDocument other) {
41+
runWithLock(lock, () -> this.clusterTime = greaterOf(other));
3642
}
3743

38-
public synchronized BsonDocument greaterOf(final BsonDocument other) {
39-
if (other == null) {
40-
return clusterTime;
41-
} else if (clusterTime == null) {
42-
return other;
43-
} else {
44-
return other.getTimestamp(CLUSTER_TIME_KEY).compareTo(clusterTime.getTimestamp(CLUSTER_TIME_KEY)) > 0 ? other : clusterTime;
45-
}
44+
public BsonDocument greaterOf(final BsonDocument other) {
45+
return supplyWithLock(lock, () -> {
46+
if (other == null) {
47+
return clusterTime;
48+
} else if (clusterTime == null) {
49+
return other;
50+
} else {
51+
return other.getTimestamp(CLUSTER_TIME_KEY).compareTo(clusterTime.getTimestamp(CLUSTER_TIME_KEY)) > 0 ? other : clusterTime;
52+
}
53+
});
4654
}
4755
}

‎driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@
1919
import com.mongodb.AuthenticationMechanism;
2020
import com.mongodb.MongoCredential;
2121

22+
import java.util.concurrent.locks.Lock;
23+
import java.util.concurrent.locks.ReentrantLock;
24+
25+
import static com.mongodb.internal.Locks.runWithLock;
26+
import static com.mongodb.internal.Locks.supplyWithLock;
27+
2228
public class MongoCredentialWithCache {
2329
private final MongoCredential credential;
2430
private final Cache cache;
@@ -53,21 +59,29 @@ public void putInCache(final Object key, final Object value) {
5359
cache.set(key, value);
5460
}
5561

62+
public Lock getLock() {
63+
return cache.lock;
64+
}
5665

5766
static class Cache {
67+
private final ReentrantLock lock = new ReentrantLock();
5868
private Object cacheKey;
5969
private Object cacheValue;
6070

61-
synchronized Object get(final Object key) {
62-
if (cacheKey != null && cacheKey.equals(key)) {
63-
return cacheValue;
64-
}
65-
return null;
71+
Object get(final Object key) {
72+
return supplyWithLock(lock, () -> {
73+
if (cacheKey != null && cacheKey.equals(key)) {
74+
return cacheValue;
75+
}
76+
return null;
77+
});
6678
}
6779

68-
synchronized void set(final Object key, final Object value) {
69-
cacheKey = key;
70-
cacheValue = value;
80+
void set(final Object key, final Object value) {
81+
runWithLock(lock, () -> {
82+
cacheKey = key;
83+
cacheValue = value;
84+
});
7185
}
7286
}
7387
}

‎driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@
4242

4343
import static com.mongodb.MongoCredential.JAVA_SUBJECT_KEY;
4444
import static com.mongodb.MongoCredential.JAVA_SUBJECT_PROVIDER_KEY;
45+
import static com.mongodb.internal.Locks.supplyWithLock;
4546
import static com.mongodb.internal.async.ErrorHandlingResultCallback.errorHandlingCallback;
4647
import static com.mongodb.internal.connection.CommandHelper.executeCommand;
4748
import static com.mongodb.internal.connection.CommandHelper.executeCommandAsync;
4849

4950
abstract class SaslAuthenticator extends Authenticator implements SpeculativeAuthenticator {
5051
public static final Logger LOGGER = Loggers.getLogger("authenticator");
5152
private static final String SUBJECT_PROVIDER_CACHE_KEY = "SUBJECT_PROVIDER";
52-
5353
SaslAuthenticator(final MongoCredentialWithCache credential, final ClusterConnectionMode clusterConnectionMode,
5454
final @Nullable ServerApi serverApi) {
5555
super(credential, clusterConnectionMode, serverApi);
@@ -205,7 +205,7 @@ protected Subject getSubject() {
205205

206206
@NonNull
207207
private SubjectProvider getSubjectProvider() {
208-
synchronized (getMongoCredentialWithCache()) {
208+
return supplyWithLock(getMongoCredentialWithCache().getLock(), () -> {
209209
SubjectProvider subjectProvider =
210210
getMongoCredentialWithCache().getFromCache(SUBJECT_PROVIDER_CACHE_KEY, SubjectProvider.class);
211211
if (subjectProvider == null) {
@@ -216,7 +216,7 @@ private SubjectProvider getSubjectProvider() {
216216
getMongoCredentialWithCache().putInCache(SUBJECT_PROVIDER_CACHE_KEY, subjectProvider);
217217
}
218218
return subjectProvider;
219-
}
219+
});
220220
}
221221

222222
@NonNull

0 commit comments

Comments
 (0)