Skip to content

Commit 076385f

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

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
@@ -23,6 +23,7 @@
2323
import java.util.concurrent.Future;
2424
import java.util.concurrent.TimeUnit;
2525
import java.util.concurrent.TimeoutException;
26+
import java.util.concurrent.atomic.AtomicBoolean;
2627

2728
import org.atmosphere.cpr.AtmosphereResource;
2829
import org.atmosphere.cpr.AtmosphereResource.TRANSPORT;
@@ -54,7 +55,7 @@ public class AtmospherePushConnection
5455
private transient FragmentedMessage incomingMessage;
5556
private transient Future<Object> outgoingMessage;
5657
private transient Object lock = new Object();
57-
private volatile boolean disconnecting;
58+
private AtomicBoolean disconnecting = new AtomicBoolean(false);
5859

5960
/**
6061
* Represents a message that can arrive as multiple fragments.
@@ -189,8 +190,9 @@ public void push() {
189190
* false if it is a response to a client request.
190191
*/
191192
public void push(boolean async) {
192-
if (disconnecting || !isConnected()) {
193-
if (disconnecting) {
193+
boolean isDisconnecting = disconnecting.get();
194+
if (isDisconnecting || !isConnected()) {
195+
if (isDisconnecting) {
194196
getLogger().debug(
195197
"Disconnection in progress, ignoring push request");
196198
}
@@ -325,7 +327,11 @@ public void disconnect() {
325327
// to skip the operation. This also prevents potential deadlocks if the
326328
// container acquires locks during operations on HTTP session, as
327329
// closing the AtmosphereResource may cause HTTP session access
328-
if (disconnecting) {
330+
// Atomically claim the right to disconnect. Only one thread can
331+
// pass this gate - eliminates the TOCTOU (time-of-check-time-of-use)
332+
// race that existed when the volatile boolean was checked outside
333+
// synchronized(lock) but set inside it.
334+
if (!disconnecting.compareAndSet(false, true)) {
329335
getLogger().debug(
330336
"Disconnection already in progress, ignoring request");
331337
return;
@@ -340,7 +346,6 @@ public void disconnect() {
340346
return;
341347
}
342348
try {
343-
disconnecting = true;
344349
if (resource.isResumed()) {
345350
// This can happen for long polling because of
346351
// http://dev.vaadin.com/ticket/16919
@@ -376,7 +381,7 @@ public void disconnect() {
376381
}
377382
connectionLost();
378383
} finally {
379-
disconnecting = false;
384+
disconnecting.set(false);
380385
}
381386
}
382387
}
@@ -435,7 +440,7 @@ private void readObject(ObjectInputStream stream)
435440
throws IOException, ClassNotFoundException {
436441
stream.defaultReadObject();
437442
state = State.DISCONNECTED;
438-
disconnecting = false;
443+
disconnecting = new AtomicBoolean(false);
439444
lock = new Object();
440445
}
441446

0 commit comments

Comments
 (0)