Skip to content

fix: prevent deadlock on concurrent push and disconnect (CP: 23.7) (#24229) (CP: 23.6)#24236

Merged
vaadin-bot merged 1 commit into23.6from
cherry-pick-24229-to-23.6-1777641732771
May 1, 2026
Merged

fix: prevent deadlock on concurrent push and disconnect (CP: 23.7) (#24229) (CP: 23.6)#24236
vaadin-bot merged 1 commit into23.6from
cherry-pick-24229-to-23.6-1777641732771

Conversation

@vaadin-bot
Copy link
Copy Markdown
Collaborator

This PR cherry-picks changes from the original PR #24229 to branch 23.6.

Original PR description

The previous AtomicBoolean guard in AtmospherePushConnection closed only the disconnect-vs-disconnect race. A push thread that reads disconnecting as false before a concurrent disconnect() flips it can still proceed into synchronized(lock) behind the disconnect thread, which is itself blocked inside resource.close() waiting for the servlet container's HTTP session lock held by the push thread — a two-lock cycle.

Move resource.close() out of the monitor: inside synchronized(lock) capture the resource into a local and call connectionLost() to transition the state, then release the monitor before invoking close() on the stashed reference. Add a matching re-check of isConnected() at the top of the synchronized block in push() so a push that waited for the monitor observes the late disconnect and defers via PUSH_PENDING/RESPONSE_PENDING instead of NPEing on the cleared resource. The disconnecting flag stays set until close() returns so subsequent pushes take the fast path and no new disconnect() re-enters while close() is still in flight.

Related-to #24192

…24229)

The previous `AtomicBoolean` guard in `AtmospherePushConnection` closed
only the disconnect-vs-disconnect race. A push thread that reads
`disconnecting` as false before a concurrent `disconnect()` flips it can
still proceed into `synchronized(lock)` behind the disconnect thread,
which is itself blocked inside `resource.close()` waiting for the
servlet container's HTTP session lock held by the push thread — a
two-lock cycle.

Move `resource.close()` out of the monitor: inside `synchronized(lock)`
capture the resource into a local and call `connectionLost()` to
transition the state, then release the monitor before invoking `close()`
on the stashed reference. Add a matching re-check of `isConnected()` at
the top of the `synchronized` block in `push()` so a push that waited
for the monitor observes the late disconnect and defers via
`PUSH_PENDING`/`RESPONSE_PENDING` instead of NPEing on the cleared
resource. The `disconnecting` flag stays set until `close()` returns so
subsequent pushes take the fast path and no new `disconnect()` re-enters
while `close()` is still in flight.

Related-to #24192
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 1, 2026

@vaadin-bot
Copy link
Copy Markdown
Collaborator Author

This PR is eligible for auto-merging policy, so it has been approved automatically. If there are pending conditions, auto merge (with 'squash' method) has been enabled for this PR[Message is sent from bot]

@vaadin-bot vaadin-bot enabled auto-merge (squash) May 1, 2026 13:29
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Test Results

  984 files  ± 0    984 suites  ±0   42m 14s ⏱️ +33s
6 336 tests + 1  6 286 ✅ + 1  50 💤 ±0  0 ❌ ±0 
6 636 runs   - 10  6 578 ✅  - 10  58 💤 ±0  0 ❌ ±0 

Results for commit 60c22fc. ± Comparison against base commit 983677b.

♻️ This comment has been updated with latest results.

@vaadin-bot vaadin-bot merged commit 7115588 into 23.6 May 1, 2026
57 of 63 checks passed
@vaadin-bot vaadin-bot deleted the cherry-pick-24229-to-23.6-1777641732771 branch May 1, 2026 14:38
@vaadin-bot
Copy link
Copy Markdown
Collaborator Author

This ticket/PR has been released with Vaadin 23.6.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants