Skip to content

Commit 80a9cb4

Browse files
authored
Make the ClientSession#close method thread-safe (#1179)
Replace the volatile boolean with an AtomicBoolean to ensure that the side effects of the close method only execute once. JAVA-5107
1 parent 8cdd8f6 commit 80a9cb4

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

driver-core/src/main/com/mongodb/internal/session/BaseClientSessionImpl.java

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.bson.BsonDocument;
2727
import org.bson.BsonTimestamp;
2828

29+
import java.util.concurrent.atomic.AtomicBoolean;
30+
2931
import static com.mongodb.assertions.Assertions.assertTrue;
3032
import static com.mongodb.assertions.Assertions.isTrue;
3133

@@ -39,20 +41,19 @@ public class BaseClientSessionImpl implements ClientSession {
3941
private ServerSession serverSession;
4042
private final Object originator;
4143
private final ClientSessionOptions options;
44+
private final AtomicBoolean closed = new AtomicBoolean(false);
4245
private BsonDocument clusterTime;
4346
private BsonTimestamp operationTime;
4447
private BsonTimestamp snapshotTimestamp;
4548
private ServerAddress pinnedServerAddress;
4649
private BsonDocument recoveryToken;
4750
private ReferenceCounted transactionContext;
48-
private volatile boolean closed;
4951

5052
public BaseClientSessionImpl(final ServerSessionPool serverSessionPool, final Object originator, final ClientSessionOptions options) {
5153
this.serverSessionPool = serverSessionPool;
5254
this.originator = originator;
5355
this.options = options;
5456
this.pinnedServerAddress = null;
55-
closed = false;
5657
}
5758

5859
@Override
@@ -121,7 +122,7 @@ public BsonTimestamp getOperationTime() {
121122

122123
@Override
123124
public ServerSession getServerSession() {
124-
isTrue("open", !closed);
125+
isTrue("open", !closed.get());
125126
if (serverSession == null) {
126127
serverSession = serverSessionPool.get();
127128
}
@@ -130,19 +131,19 @@ public ServerSession getServerSession() {
130131

131132
@Override
132133
public void advanceOperationTime(@Nullable final BsonTimestamp newOperationTime) {
133-
isTrue("open", !closed);
134+
isTrue("open", !closed.get());
134135
this.operationTime = greaterOf(newOperationTime);
135136
}
136137

137138
@Override
138139
public void advanceClusterTime(@Nullable final BsonDocument newClusterTime) {
139-
isTrue("open", !closed);
140+
isTrue("open", !closed.get());
140141
this.clusterTime = greaterOf(newClusterTime);
141142
}
142143

143144
@Override
144145
public void setSnapshotTimestamp(@Nullable final BsonTimestamp snapshotTimestamp) {
145-
isTrue("open", !closed);
146+
isTrue("open", !closed.get());
146147
if (snapshotTimestamp != null) {
147148
if (this.snapshotTimestamp != null && !snapshotTimestamp.equals(this.snapshotTimestamp)) {
148149
throw new MongoClientException("Snapshot timestamps should not change during the lifetime of the session. Current "
@@ -155,7 +156,7 @@ public void setSnapshotTimestamp(@Nullable final BsonTimestamp snapshotTimestamp
155156
@Override
156157
@Nullable
157158
public BsonTimestamp getSnapshotTimestamp() {
158-
isTrue("open", !closed);
159+
isTrue("open", !closed.get());
159160
return snapshotTimestamp;
160161
}
161162

@@ -182,8 +183,10 @@ private BsonTimestamp greaterOf(@Nullable final BsonTimestamp newOperationTime)
182183

183184
@Override
184185
public void close() {
185-
if (!closed) {
186-
closed = true;
186+
// While the interface implemented by this class is documented as not thread safe, it's still useful to provide thread safety here
187+
// in order to prevent the code within the conditional from executing more than once. Doing so protects the server session pool from
188+
// corruption, by preventing the same server session from being released to the pool more than once.
189+
if (closed.compareAndSet(false, true)) {
187190
if (serverSession != null) {
188191
serverSessionPool.release(serverSession);
189192
}

0 commit comments

Comments
 (0)