Skip to content

Commit a7176db

Browse files
fix: fix potential deadlock in AtmospherePushConnection disconnect (#24133) (CP: 24.9) (#24160)
This PR cherry-picks changes from the original PR #24133 to branch 24.9. --- #### Original PR description > Fixes #24129 Co-authored-by: Marco Collovati <marco@vaadin.com>
1 parent 1f0120e commit a7176db

1 file changed

Lines changed: 12 additions & 7 deletions

File tree

flow-server/src/main/java/com/vaadin/flow/server/communication/AtmospherePushConnection.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.concurrent.Future;
2525
import java.util.concurrent.TimeUnit;
2626
import java.util.concurrent.TimeoutException;
27+
import java.util.concurrent.atomic.AtomicBoolean;
2728

2829
import com.fasterxml.jackson.databind.JsonNode;
2930
import org.atmosphere.cpr.AtmosphereResource;
@@ -55,7 +56,7 @@ public class AtmospherePushConnection
5556
private transient FragmentedMessage incomingMessage;
5657
private transient Future<Object> outgoingMessage;
5758
private transient Object lock = new Object();
58-
private volatile boolean disconnecting;
59+
private AtomicBoolean disconnecting = new AtomicBoolean(false);
5960

6061
/**
6162
* Represents a message that can arrive as multiple fragments.
@@ -190,8 +191,9 @@ public void push() {
190191
* false if it is a response to a client request.
191192
*/
192193
public void push(boolean async) {
193-
if (disconnecting || !isConnected()) {
194-
if (disconnecting) {
194+
boolean isDisconnecting = disconnecting.get();
195+
if (isDisconnecting || !isConnected()) {
196+
if (isDisconnecting) {
195197
getLogger().debug(
196198
"Disconnection in progress, ignoring push request");
197199
}
@@ -324,7 +326,11 @@ public void disconnect() {
324326
// to skip the operation. This also prevents potential deadlocks if the
325327
// container acquires locks during operations on HTTP session, as
326328
// closing the AtmosphereResource may cause HTTP session access
327-
if (disconnecting) {
329+
// Atomically claim the right to disconnect. Only one thread can
330+
// pass this gate - eliminates the TOCTOU (time-of-check-time-of-use)
331+
// race that existed when the volatile boolean was checked outside
332+
// synchronized(lock) but set inside it.
333+
if (!disconnecting.compareAndSet(false, true)) {
328334
getLogger().debug(
329335
"Disconnection already in progress, ignoring request");
330336
return;
@@ -339,7 +345,6 @@ public void disconnect() {
339345
return;
340346
}
341347
try {
342-
disconnecting = true;
343348
if (resource.isResumed()) {
344349
// This can happen for long polling because of
345350
// http://dev.vaadin.com/ticket/16919
@@ -375,7 +380,7 @@ public void disconnect() {
375380
}
376381
connectionLost();
377382
} finally {
378-
disconnecting = false;
383+
disconnecting.set(false);
379384
}
380385
}
381386
}
@@ -434,7 +439,7 @@ private void readObject(ObjectInputStream stream)
434439
throws IOException, ClassNotFoundException {
435440
stream.defaultReadObject();
436441
state = State.DISCONNECTED;
437-
disconnecting = false;
442+
disconnecting = new AtomicBoolean(false);
438443
lock = new Object();
439444
}
440445

0 commit comments

Comments
 (0)