Skip to content

Commit 48def21

Browse files
authored
fix: fix potential deadlock in AtmospherePushConnection disconnect (CP: 2.13) (#24155)
Fixes #24192
1 parent 531109e commit 48def21

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
@@ -17,6 +17,7 @@
1717
import java.util.concurrent.Future;
1818
import java.util.concurrent.TimeUnit;
1919
import java.util.concurrent.TimeoutException;
20+
import java.util.concurrent.atomic.AtomicBoolean;
2021

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

5354
/**
5455
* Represents a message that can arrive as multiple fragments.
@@ -181,8 +182,9 @@ public void push() {
181182
* false if it is a response to a client request.
182183
*/
183184
public void push(boolean async) {
184-
if (disconnecting || !isConnected()) {
185-
if (disconnecting) {
185+
boolean isDisconnecting = disconnecting.get();
186+
if (isDisconnecting || !isConnected()) {
187+
if (isDisconnecting) {
186188
getLogger().debug(
187189
"Disconnection in progress, ignoring push request");
188190
}
@@ -315,7 +317,11 @@ public void disconnect() {
315317
// to skip the operation. This also prevents potential deadlocks if the
316318
// container acquires locks during operations on HTTP session, as
317319
// closing the AtmosphereResource may cause HTTP session access
318-
if (disconnecting) {
320+
// Atomically claim the right to disconnect. Only one thread can
321+
// pass this gate - eliminates the TOCTOU (time-of-check-time-of-use)
322+
// race that existed when the volatile boolean was checked outside
323+
// synchronized(lock) but set inside it.
324+
if (!disconnecting.compareAndSet(false, true)) {
319325
getLogger().debug(
320326
"Disconnection already in progress, ignoring request");
321327
return;
@@ -330,7 +336,6 @@ public void disconnect() {
330336
return;
331337
}
332338
try {
333-
disconnecting = true;
334339
if (resource.isResumed()) {
335340
// This can happen for long polling because of
336341
// http://dev.vaadin.com/ticket/16919
@@ -361,7 +366,7 @@ public void disconnect() {
361366
}
362367
connectionLost();
363368
} finally {
364-
disconnecting = false;
369+
disconnecting.set(false);
365370
}
366371
}
367372
}
@@ -406,7 +411,7 @@ private void readObject(ObjectInputStream stream)
406411
throws IOException, ClassNotFoundException {
407412
stream.defaultReadObject();
408413
state = State.DISCONNECTED;
409-
disconnecting = false;
414+
disconnecting = new AtomicBoolean(false);
410415
lock = new Object();
411416
}
412417

0 commit comments

Comments
 (0)