Skip to content

fix: resolve route scope UI from the detach event (CP: 24.10) - #25069

Merged
caalador merged 2 commits into
24.10from
fix/route-scope-stale-detach-listener-24.10
Jul 30, 2026
Merged

fix: resolve route scope UI from the detach event (CP: 24.10)#25069
caalador merged 2 commits into
24.10from
fix/route-scope-stale-detach-listener-24.10

Conversation

@mcollovati

Copy link
Copy Markdown
Collaborator

This PR cherry-picks changes from the original PR #25064 to branch 24.10.

Original PR description

RouteStoreWrapper#getBeanStore re-points a bean store to the UI created by a
browser refresh, but the store's detach listener stays on the previous UI.
resetUI() then asked findPreservingUI() about currentUI, which is no longer
the UI the detach event comes from.

If the refreshed UI had already been removed from the session, its session
reference is null and findPreservingUI() threw a NullPointerException. The
exception escaped before the branch that destroys the store, so the route
scoped beans were not destroyed and the store stayed in routeStores. During
session expiration the beans are still released by the session destroy
listener of RouteStoreWrapper, but when only the UIs of an idle browser
window are removed nothing cleans them up and they leak until the session
ends.

Pass the detached UI from the event into resetUI() so the lookup always gets
the UI the event originates from, which still has its session at that point.
findPreservingUI() now also returns null instead of dereferencing a UI that
has no session anymore.

Fixes #25027

RouteStoreWrapper#getBeanStore re-points a bean store to the UI created by a
browser refresh, but the store's detach listener stays on the previous UI.
resetUI() then asked findPreservingUI() about currentUI, which is no longer
the UI the detach event comes from.

If the refreshed UI had already been removed from the session, its session
reference is null and findPreservingUI() threw a NullPointerException. The
exception escaped before the branch that destroys the store, so the route
scoped beans were not destroyed and the store stayed in routeStores. During
session expiration the beans are still released by the session destroy
listener of RouteStoreWrapper, but when only the UIs of an idle browser
window are removed nothing cleans them up and they leak until the session
ends.

Pass the detached UI from the event into resetUI() so the lookup always gets
the UI the event originates from, which still has its session at that point.
findPreservingUI() now also returns null instead of dereferencing a UI that
has no session anymore.

Fixes #25027
mockSession() sets the current VaadinSession in a thread local that nothing
cleared once the test had finished. Surefire reuses the JVM and its threads
across test classes, so the stale session stayed visible to unrelated tests.

VaadinSessionScopeTest extends AbstractScopeTest directly and therefore has
no setup clearing the thread local, while inheriting tests that assert an
exception is thrown when no session is current. Those tests passed only
because of the order the test methods happened to run in.
@github-actions

Copy link
Copy Markdown
Contributor

Test Results

1 295 files  ±0  1 295 suites  ±0   1h 10m 26s ⏱️ - 1m 55s
8 954 tests +2  8 892 ✅ +2  62 💤 ±0  0 ❌ ±0 
9 338 runs   - 6  9 269 ✅  - 6  69 💤 ±0  0 ❌ ±0 

Results for commit 36b2306. ± Comparison against base commit b43131f.

@caalador
caalador merged commit 677f145 into 24.10 Jul 30, 2026
26 checks passed
@caalador
caalador deleted the fix/route-scope-stale-detach-listener-24.10 branch July 30, 2026 03:59
vaadin-bot added a commit that referenced this pull request Jul 30, 2026
… (CP: 24.9) (#25072)

This PR cherry-picks changes from the original PR #25069 to branch 24.9.
---
#### Original PR description
> This PR cherry-picks changes from the original PR #25064 to branch
24.10.
> ---
> #### Original PR description
> > RouteStoreWrapper#getBeanStore re-points a bean store to the UI
created by a
> > browser refresh, but the store's detach listener stays on the
previous UI.
> > resetUI() then asked findPreservingUI() about currentUI, which is no
longer
> > the UI the detach event comes from.
> > 
> > If the refreshed UI had already been removed from the session, its
session
> > reference is null and findPreservingUI() threw a
NullPointerException. The
> > exception escaped before the branch that destroys the store, so the
route
> > scoped beans were not destroyed and the store stayed in routeStores.
During
> > session expiration the beans are still released by the session
destroy
> > listener of RouteStoreWrapper, but when only the UIs of an idle
browser
> > window are removed nothing cleans them up and they leak until the
session
> > ends.
> > 
> > Pass the detached UI from the event into resetUI() so the lookup
always gets
> > the UI the event originates from, which still has its session at
that point.
> > findPreservingUI() now also returns null instead of dereferencing a
UI that
> > has no session anymore.
> > 
> > Fixes #25027

Co-authored-by: Marco Collovati <marco@vaadin.com>
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