Skip to content

Commit 51b4e65

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

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
@@ -16,6 +16,7 @@
1616
import java.util.concurrent.Future;
1717
import java.util.concurrent.TimeUnit;
1818
import java.util.concurrent.TimeoutException;
19+
import java.util.concurrent.atomic.AtomicBoolean;
1920

2021
import org.atmosphere.cpr.AtmosphereResource;
2122
import org.atmosphere.cpr.AtmosphereResource.TRANSPORT;
@@ -47,7 +48,7 @@ public class AtmospherePushConnection implements PushConnection {
4748
private transient FragmentedMessage incomingMessage;
4849
private transient Future<Object> outgoingMessage;
4950
private transient Object lock = new Object();
50-
private volatile boolean disconnecting;
51+
private AtomicBoolean disconnecting = new AtomicBoolean(false);
5152

5253
/**
5354
* Represents a message that can arrive as multiple fragments.
@@ -180,8 +181,9 @@ public void push() {
180181
* false if it is a response to a client request.
181182
*/
182183
public void push(boolean async) {
183-
if (disconnecting || !isConnected()) {
184-
if (disconnecting) {
184+
boolean isDisconnecting = disconnecting.get();
185+
if (isDisconnecting || !isConnected()) {
186+
if (isDisconnecting) {
185187
getLogger().debug(
186188
"Disconnection in progress, ignoring push request");
187189
}
@@ -314,7 +316,11 @@ public void disconnect() {
314316
// to skip the operation. This also prevents potential deadlocks if the
315317
// container acquires locks during operations on HTTP session, as
316318
// closing the AtmosphereResource may cause HTTP session access
317-
if (disconnecting) {
319+
// Atomically claim the right to disconnect. Only one thread can
320+
// pass this gate - eliminates the TOCTOU (time-of-check-time-of-use)
321+
// race that existed when the volatile boolean was checked outside
322+
// synchronized(lock) but set inside it.
323+
if (!disconnecting.compareAndSet(false, true)) {
318324
getLogger().debug(
319325
"Disconnection already in progress, ignoring request");
320326
return;
@@ -329,7 +335,6 @@ public void disconnect() {
329335
return;
330336
}
331337
try {
332-
disconnecting = true;
333338
if (resource.isResumed()) {
334339
// This can happen for long polling because of
335340
// http://dev.vaadin.com/ticket/16919
@@ -365,7 +370,7 @@ public void disconnect() {
365370
}
366371
connectionLost();
367372
} finally {
368-
disconnecting = false;
373+
disconnecting.set(false);
369374
}
370375
}
371376
}
@@ -410,7 +415,7 @@ private void readObject(ObjectInputStream stream)
410415
throws IOException, ClassNotFoundException {
411416
stream.defaultReadObject();
412417
state = State.DISCONNECTED;
413-
disconnecting = false;
418+
disconnecting = new AtomicBoolean(false);
414419
lock = new Object();
415420
}
416421

0 commit comments

Comments
 (0)