Skip to content

Commit cc56a93

Browse files
christophstroblchristophstrobl
committed
Refine locking.
Closes: #4429 Original Pull Request: #4431
1 parent fd96974 commit cc56a93

File tree

4 files changed

+139
-89
lines changed

4 files changed

+139
-89
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@
1818
import java.util.Collection;
1919
import java.util.List;
2020
import java.util.Set;
21+
import java.util.concurrent.atomic.AtomicReference;
22+
import java.util.concurrent.locks.Lock;
23+
import java.util.concurrent.locks.ReentrantLock;
2124
import java.util.function.Consumer;
2225
import java.util.function.Supplier;
26+
import java.util.function.UnaryOperator;
2327
import java.util.stream.Stream;
2428

2529
import org.bson.Document;
@@ -188,16 +192,19 @@ default SessionScoped withSession(Supplier<ClientSession> sessionProvider) {
188192

189193
return new SessionScoped() {
190194

191-
private final Object lock = new Object();
192-
private @Nullable ClientSession session = null;
195+
private final Lock lock = new ReentrantLock();
196+
private @Nullable ClientSession session;
193197

194198
@Override
195199
public <T> T execute(SessionCallback<T> action, Consumer<ClientSession> onComplete) {
196200

197-
synchronized (lock) {
201+
lock.lock();
202+
try {
198203
if (session == null) {
199204
session = sessionProvider.get();
200205
}
206+
} finally {
207+
lock.unlock();
201208
}
202209

203210
try {

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/LazyLoadingProxyFactory.java‎

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
import java.io.ObjectOutputStream;
2323
import java.io.Serializable;
2424
import java.lang.reflect.Method;
25+
import java.util.concurrent.locks.Lock;
26+
import java.util.concurrent.locks.ReadWriteLock;
27+
import java.util.concurrent.locks.ReentrantReadWriteLock;
2528
import java.util.function.Supplier;
2629

2730
import org.aopalliance.intercept.MethodInterceptor;
@@ -134,7 +137,8 @@ public Object createLazyLoadingProxy(MongoPersistentProperty property, DbRefReso
134137
}
135138

136139
return prepareProxyFactory(propertyType,
137-
() -> new LazyLoadingInterceptor(property, callback, source, exceptionTranslator)).getProxy(LazyLoadingProxy.class.getClassLoader());
140+
() -> new LazyLoadingInterceptor(property, callback, source, exceptionTranslator))
141+
.getProxy(LazyLoadingProxy.class.getClassLoader());
138142
}
139143

140144
/**
@@ -171,6 +175,8 @@ public static class LazyLoadingInterceptor
171175
}
172176
}
173177

178+
private final ReadWriteLock lock = new ReentrantReadWriteLock();
179+
174180
private final MongoPersistentProperty property;
175181
private final DbRefResolverCallback callback;
176182
private final Object source;
@@ -339,25 +345,29 @@ private void readObject(ObjectInputStream in) throws IOException {
339345
}
340346

341347
@Nullable
342-
private synchronized Object resolve() {
348+
private Object resolve() {
343349

344-
if (resolved) {
350+
lock.readLock().lock();
351+
try {
352+
if (resolved) {
345353

346-
if (LOGGER.isTraceEnabled()) {
347-
LOGGER.trace(String.format("Accessing already resolved lazy loading property %s.%s",
348-
property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
354+
if (LOGGER.isTraceEnabled()) {
355+
LOGGER.trace(String.format("Accessing already resolved lazy loading property %s.%s",
356+
property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
357+
}
358+
return result;
349359
}
350-
return result;
360+
} finally {
361+
lock.readLock().unlock();
351362
}
352363

353-
try {
354-
if (LOGGER.isTraceEnabled()) {
355-
LOGGER.trace(String.format("Resolving lazy loading property %s.%s",
356-
property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
357-
}
358-
359-
return callback.resolve(property);
364+
if (LOGGER.isTraceEnabled()) {
365+
LOGGER.trace(String.format("Resolving lazy loading property %s.%s",
366+
property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
367+
}
360368

369+
try {
370+
return executeWhileLocked(lock.writeLock(), () -> callback.resolve(property));
361371
} catch (RuntimeException ex) {
362372

363373
DataAccessException translatedException = exceptionTranslator.translateExceptionIfPossible(ex);
@@ -370,6 +380,16 @@ private synchronized Object resolve() {
370380
translatedException != null ? translatedException : ex);
371381
}
372382
}
383+
384+
private static <T> T executeWhileLocked(Lock lock, Supplier<T> stuff) {
385+
386+
lock.lock();
387+
try {
388+
return stuff.get();
389+
} finally {
390+
lock.unlock();
391+
}
392+
}
373393
}
374394

375395
}

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/messaging/CursorReadingTask.java‎

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import java.time.Duration;
1919
import java.util.concurrent.CountDownLatch;
2020
import java.util.concurrent.TimeUnit;
21+
import java.util.concurrent.locks.Lock;
22+
import java.util.concurrent.locks.ReentrantLock;
2123
import java.util.function.Supplier;
2224

2325
import org.springframework.dao.DataAccessResourceFailureException;
@@ -39,7 +41,7 @@
3941
*/
4042
abstract class CursorReadingTask<T, R> implements Task {
4143

42-
private final Object lifecycleMonitor = new Object();
44+
private final Lock lock = new ReentrantLock();
4345

4446
private final MongoTemplate template;
4547
private final SubscriptionRequest<T, R, RequestOptions> request;
@@ -86,19 +88,14 @@ public void run() {
8688
}
8789
} catch (InterruptedException e) {
8890

89-
synchronized (lifecycleMonitor) {
90-
state = State.CANCELLED;
91-
}
91+
doWhileLocked(lock, () -> state = State.CANCELLED);
9292
Thread.currentThread().interrupt();
9393
break;
9494
}
9595
}
9696
} catch (RuntimeException e) {
9797

98-
synchronized (lifecycleMonitor) {
99-
state = State.CANCELLED;
100-
}
101-
98+
doWhileLocked(lock, () -> state = State.CANCELLED);
10299
errorHandler.handleError(e);
103100
}
104101
}
@@ -114,40 +111,40 @@ public void run() {
114111
*/
115112
private void start() {
116113

117-
synchronized (lifecycleMonitor) {
114+
doWhileLocked(lock, () -> {
118115
if (!State.RUNNING.equals(state)) {
119116
state = State.STARTING;
120117
}
121-
}
118+
});
122119

123120
do {
124121

125-
boolean valid = false;
122+
// boolean valid = false;
126123

127-
synchronized (lifecycleMonitor) {
124+
boolean valid = executeWhileLocked(lock, () -> {
128125

129-
if (State.STARTING.equals(state)) {
126+
if (!State.STARTING.equals(state)) {
127+
return false;
128+
}
130129

131-
MongoCursor<T> cursor = execute(() -> initCursor(template, request.getRequestOptions(), targetType));
132-
valid = isValidCursor(cursor);
133-
if (valid) {
134-
this.cursor = cursor;
135-
state = State.RUNNING;
136-
} else if (cursor != null) {
137-
cursor.close();
138-
}
130+
MongoCursor<T> cursor = execute(() -> initCursor(template, request.getRequestOptions(), targetType));
131+
boolean isValid = isValidCursor(cursor);
132+
if (isValid) {
133+
this.cursor = cursor;
134+
state = State.RUNNING;
135+
} else if (cursor != null) {
136+
cursor.close();
139137
}
140-
}
138+
return isValid;
139+
});
141140

142141
if (!valid) {
143142

144143
try {
145144
Thread.sleep(100);
146145
} catch (InterruptedException e) {
147146

148-
synchronized (lifecycleMonitor) {
149-
state = State.CANCELLED;
150-
}
147+
doWhileLocked(lock, () -> state = State.CANCELLED);
151148
Thread.currentThread().interrupt();
152149
}
153150
}
@@ -163,15 +160,15 @@ private void start() {
163160
@Override
164161
public void cancel() throws DataAccessResourceFailureException {
165162

166-
synchronized (lifecycleMonitor) {
163+
doWhileLocked(lock, () -> {
167164

168165
if (State.RUNNING.equals(state) || State.STARTING.equals(state)) {
169166
this.state = State.CANCELLED;
170167
if (cursor != null) {
171168
cursor.close();
172169
}
173170
}
174-
}
171+
});
175172
}
176173

177174
@Override
@@ -181,10 +178,7 @@ public boolean isLongLived() {
181178

182179
@Override
183180
public State getState() {
184-
185-
synchronized (lifecycleMonitor) {
186-
return state;
187-
}
181+
return executeWhileLocked(lock, () -> state);
188182
}
189183

190184
@Override
@@ -220,13 +214,12 @@ private void emitMessage(Message<T, R> message) {
220214
@Nullable
221215
private T getNext() {
222216

223-
synchronized (lifecycleMonitor) {
217+
return executeWhileLocked(lock, () -> {
224218
if (State.RUNNING.equals(state)) {
225219
return cursor.tryNext();
226220
}
227-
}
228-
229-
throw new IllegalStateException(String.format("Cursor %s is not longer open", cursor));
221+
throw new IllegalStateException(String.format("Cursor %s is not longer open", cursor));
222+
});
230223
}
231224

232225
private static boolean isValidCursor(@Nullable MongoCursor<?> cursor) {
@@ -263,4 +256,23 @@ private <V> V execute(Supplier<V> callback) {
263256
throw translated != null ? translated : e;
264257
}
265258
}
259+
260+
private static void doWhileLocked(Lock lock, Runnable action) {
261+
262+
executeWhileLocked(lock, () -> {
263+
action.run();
264+
return null;
265+
});
266+
}
267+
268+
@Nullable
269+
private static <T> T executeWhileLocked(Lock lock, Supplier<T> stuff) {
270+
271+
lock.lock();
272+
try {
273+
return stuff.get();
274+
} finally {
275+
lock.unlock();
276+
}
277+
}
266278
}

0 commit comments

Comments
 (0)