Feature/component garbage collection#138
Open
janvojt wants to merge 8 commits into
Open
Conversation
Components used to stay strongly referenced in SessionContext's registry for the whole session lifetime, so long-lived browser tabs accumulated every component ever rendered and eventually caused server OOM (the browser-side registry leaked identically). The registry now holds weak references: while a component is displayed (reachable from a root panel, shown window/popup/notification, context menu, toolbar dropdown) or referenced by application code, it stays alive; once unreachable, the JVM collects it and a ReferenceQueue drain sends UiRootPanel.destroyComponent so the client frees its DOM and registry entry too. Draining happens on component registration and every 10 seconds in TeamAppsSessionManager's housekeeping loop. Gated by TeamAppsConfiguration.clientObjectGarbageCollectionEnabled, default OFF. With the flag off, behavior is strictly historical (every registered component is pinned; unpin is a no-op), with one deliberate exception: events/queries for unknown component ids now log a warning instead of throwing TeamAppsComponentNotFoundException, which used to destroy the entire session. With the flag ON, note two behavior changes: - render-and-forget of NOT-displayed components breaks: apps must hold a Java reference to any component they want to reuse later - custom components referencing a temporary component only through a queued command must pin it via the new SessionContext.pinClientObject/unpinClientObject escape hatch New observability: SessionContext.getClientObjectCount() / getCollectedClientObjectsCount(), surfaced in UiSessionStats. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntics with a test The client-side UiRootPanel.buildRootPanel appends the root panel to the container element instead of replacing an existing one, so every component attached via SessionContext.addRootPanel stays displayed for the rest of the session and must stay strongly referenced. Clarify this in the Javadoc of attachedRootComponents (it must not be keyed by container selector) and add a test asserting that two root panels added for the same selector both stay referenced under client-object garbage collection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hen GC is disabled The client-object GC commit unconditionally changed handling of UI events/queries referencing unknown client object ids from fail-fast to warn-and-continue. That tolerant path is only justified when garbage collection is enabled, where an event/query can legitimately race with collection of its target. With the flag off (the default), an unknown id indicates a genuine client/server desync, so the historical fail-fast behavior is restored: TeamAppsComponentNotFoundException is thrown and the session is destroyed with SERVER_SIDE_ERROR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SessionContext.showNotification() added a SelfDisposingEventListener to notification.onClosed on every call, but a server-initiated Notification.close() never fires onClosed, so repeated show/close cycles on the same Notification instance accumulated listeners. Move the pin/unpin bookkeeping into Notification itself: showNotification() pins via the idempotent pinUntilClosed(), and Notification unpins exactly once per cycle from either close path (close() or UI_NOTIFICATION_CLOSED), without registering any per-show listeners. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- SessionContext: reuse the uxJacksonSerializationTemplate field in onUiQuery instead of allocating a new template per query - Extend the lastContextMenuComponent field comment in Table, Tree, InfiniteItemView, InfiniteItemView2, MediaSoupV3WebRtcClient and ChatDisplay to document the deliberate retention semantics - TeamAppsSessionManager: note that client object counts may lag one housekeeping cycle behind the asynchronous drain - SessionContextClientObjectGcTest: make the pre-collection assertion robust against natural GC, shut down created session executors after each test, and document the System.gc() dependence - TeamAppsConfiguration: document that listener registrations on application-scoped objects keep components strongly reachable - ToolbarButton: document the drop-down component supplier caching Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntObject Pinning was set-based, so an object pinned for two independent reasons (e.g. a shown popup additionally pinned by application code) was fully released by a single unpin, allowing a still-displayed component to be garbage collected and destroyed client-side. Pins are now reference- counted in SessionContext (saturating at zero), and Window, Popup and Notification guard their show/close pin with a pinnedWhileShowing flag so each show/close cycle pins and unpins exactly once, even on double-close, close-without-show, or a racing client-side close event. The stacking one-shot onClosed listener in showNotification is replaced by direct unpinning in Notification.handleUiEvent. Also adds the missing CurrentSessionContext.throwIfNotSameAs guard to SessionContext.unregisterClientObject, which mutates the non-thread-safe pin structure and registry like the other guarded mutators. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
UiPopup gains a `closed` DTO event, mirroring UiWindow's. The client UiPopup now actually implements closeOnEscape / closeOnClickOutside — these config flags existed and were transmitted to the client, but were dead there (no client-side close behavior at all). A user-initiated close (escape key or click outside the content) removes the popup and fires the new event; the server-side Popup handles UI_POPUP_CLOSED by unpinning itself from the SessionContext client-object registry (the pin created in SessionContext.showPopup*()) and firing its new public onClosed event. Without this, a client-closed popup would have stayed strongly referenced (pinned) until session death when client-object garbage collection is enabled. A server-initiated Popup.close() after a client-side close remains harmless: pinned client objects are held in a Set, so the double unpin is a no-op (covered by a new test). Note: the TypeScript changes could not be compiled or tested on this machine (the frontend yarn build is broken here by read-only bind mounts); they mirror the existing UiWindow.ts escape/click-outside implementation as closely as possible and should be smoke-tested on a machine with a working frontend build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Window, Popup, and Notification each duplicated the same pinnedWhileShowing guard protecting the reference-counted pin registry from double-release (e.g. a server-side close racing the client's close event). Move the flag and the pin/release pair into AbstractComponent as pinWhileDisplayed() / releaseDisplayPin(), so the invariant lives in one place and custom component authors get a documented keep-alive facility for components referenced only by the client. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Garbage collection of unused components within live sessions
Problem
SessionContextkeeps a strong reference to every client object (component) ever rendered, for the whole lifetime of the session. In long-lived sessions that repeatedly rebuild parts of their UI (dashboards, wizards, record detail views, …), all discarded component trees accumulate on the server and on the client, since the client-side counterparts are never destroyed either. Memory usage grows monotonically until the session ends.Solution
This PR makes the session's client-object registry weak-reference based, so components that are neither displayed nor referenced by application code get garbage collected during the session's lifetime, and their client-side counterparts are destroyed automatically.
The feature is opt-in via a new configuration flag (default off):
With the flag off, behavior is unchanged: every registered client object is pinned at registration time and stays strongly referenced until the session is destroyed (exact historical behavior, covered by dedicated tests).
How it works
SessionContext.clientObjectsByIdnow holdsWeakReferences (aConcurrentHashMapkeyed by component id) backed by aReferenceQueue. Collected entries are drained opportunistically on everyregisterClientObject(...)call and periodically by the session manager's housekeeping loop (which also covers idle sessions). For each drained entry, aDestroyComponentCommandis sent so the client releases the DOM/state too. A two-argremove(key, reference)guards against races with explicit unregistration — an explicitly unrendered component is never destroyed twice.addRootPanel*) — permanently, since the client appends root panels and never removes them,Windows,Popups andNotifications — pinned on show, unpinned on close (whether server-initiated or user-initiated on the client),Table,Tree,InfiniteItemView/2,ChatDisplay,MediaSoupV3WebRtcClient) andToolbarButtondrop-down content — retained by the owning component while the client may still display it,SessionContext.pinClientObject(...)/unpinClientObject(...)are public API and reference-counted, so an object pinned for multiple independent reasons stays pinned until every reason released it. Spurious unpins are no-ops (saturating at zero).Window/Popup/Notificationacquire their show-pin through the new idempotentAbstractComponent.pinWhileDisplayed()/releaseDisplayPin()helpers, so racing double-releases (e.g. a server-sideclose()plus the client's closed event) cannot underflow another pin. Custom component authors can use the same helpers for transient components that are referenced only from the client.null. With GC disabled, the historical fail-fast behavior is kept:TeamAppsComponentNotFoundExceptiondestroys the session, since an unknown id then indicates a real client/server desync.UiSessionStatsgains two values, updated by the housekeeping loop:clientObjectCount(currently registered, gauge) andcollectedClientObjectCount(collected during the session's lifetime, cumulative).Popup close-on-escape / close-on-click-outside
Popup.closeOnEscapeandcloseOnClickOutsideexisted in the API and DTO but were never implemented on the client — the flags were dead. Correct unpinning requires the server to learn about user-initiated closes, so this PR implements them:UiPopup.tsnow closes on escape / click-outside (mirroringUiWindow) and fires a newclosedDTO event; the server unpins and fires the newPopup.onClosedevent.Behavior changes to be aware of (even with the flag off)
Popup.onClosedis available.ToolbarButtondrop-down suppliers are cached: the supplier's result is reused across re-render cycles instead of being re-invoked (the client requests the content only once anyway). UseupdateDropDownComponent(...)to replace displayed content.SessionContext's constructor takes an additional boolean andUiSessionStatshas two new methods — relevant only to code reaching into framework internals.Adopting garbage collection in applications
Nothing is required for standard usage — the framework pins everything it displays itself. To benefit from the flag and stay safe:
Eventstrongly references its listeners, and a listener typically references its component — such components cannot be collected until the listener is removed (use theDisposablereturned byEvent.addListener(...)). This is the most common reason forcollectedClientObjectCountstaying near zero.createUiReference()of transient components. If the only remaining reference lives in client-side state, pin the component while it is displayed (pinWhileDisplayed()/pinClientObject(...)) and release it on close, asWindow/Popup/Notificationdo.getClientObject(id)may returnnullonce GC is enabled. Custom client-side components should tolerate anullquery result.Recommended rollout: enable in staging, watch the two new session stats, and exercise the main UI flows.
Testing
SessionContextClientObjectGcTest(22 tests): unreferenced components are collected and destroyed exactly once on the client; displayed UI (root panels, shown windows/popups/notifications) survives GC and becomes collectable after each close path (server-side and client-side); reference-counting semantics (double pin/unpin, spurious unpin, close/closed-event races); registry stays bounded under repeated content replacement; unknown-id tolerance with the flag on and fail-fast with the flag off; exact historical behavior with the flag off.teamapps-uxmodule: 251 tests, 0 failures.System.gc()triggering a collection and will not work under-XX:+DisableExplicitGC.UiPopup.tschanges mirrorUiWindow.tsbut were authored on a machine without a working frontend build — please smoke-test popup escape/click-outside closing during review.🤖 Generated with Claude Code