feat: keep a UI alive for its ongoing transfers and terminate them when the session is invalidated by totally-not-ai[bot] · Pull Request #25431 · vaadin/flow

totally-not-ai · 2026-09-02T06:23:56Z

Summary

Uploads and downloads are now tracked per UI. A UI that is closed while a transfer for it is running stays attached to its session until that request is done, so progress listeners and completion callbacks still run. When the session is invalidated or times out, its ongoing transfers are stopped instead of running to the end.

Fixes #25409

What changed

Behavior change 1 — a closed UI is detached later than before. Affects every application that serves uploads or downloads. If the user closes the browser tab (or the UI is closed for any other reason) while a transfer for that UI is being served, the UI is no longer removed from the session by the end-of-request cleanup. It is kept until the request that serves the last transfer finishes, and that request's own cleanup detaches it. Before this, the callbacks bound to that UI (TransferProgressListener, UploadHandler.toTempFile and friends) either did nothing or failed with a UIDetachedException.

Behavior change 2 — an invalidated session now kills its transfers. Affects applications whose uploads or downloads can outlive the session, for example a long download that is still running when the session times out. VaadinService now calls UIInternals.terminateActiveTransfers() for each UI of the session being destroyed. Every stream the framework hands to the request handler — the response output stream and writer, the request input stream and reader, and the streams of multipart parts — then refuses to transfer more bytes and throws an IOException. A download ends up truncated; a partly received upload is discarded and its callback is not run. A warning with the request path and the owner component is logged for each terminated transfer. An application that needs a transfer to survive can keep the session alive, or serve it from its own servlet.

Behavior change 3 — a progress notification for a detached UI no longer fails the transfer. TransferProgressAwareHandler now catches UIDetachedException from UI.access and logs it at debug level. This only happens when the session is gone, and the transfer is being terminated anyway, so the application sees the real cause instead of a detached-UI error.

How it works: StreamRequestHandler creates an ActiveTransfer per request, registers it on the UI while the session lock is held, and passes the handler a wrapped request and response. The wrappers are new subclasses of VaadinServletRequest / VaadinServletResponse, so existing casts to the servlet types keep working. A request or response that is not servlet based is passed through unwrapped, with a one-time warning per type; for those, only transfers going through TransferUtil stop.

Javadoc on UI.close(), UploadHandler, DownloadHandler, ElementRequestHandler.handleRequest and TransferProgressListener was updated to describe this lifecycle. No public API was removed or renamed.

Use case

An app lets users upload a large CSV of invoices. The import runs in the completion callback, and the app shows the result in the UI. Users often close the tab once the progress bar looks full — before this change, the upload reached the server but the callback was skipped or threw, so the import silently never happened. Now the upload finishes and the import runs; only losing the whole session (logout or timeout) cancels it.

UploadHandler handler = UploadHandler
        .toTempFile((metadata, file) -> invoiceImporter.importFrom(file))
        .onProgress((transferred, total) -> progressBar.setValue(transferred))
        .whenComplete(success -> statusLabel
                .setText(success ? "Import done" : "Import cancelled"));

uploadElement.setAttribute("target", handler);

API Changes

com.vaadin.flow.internal.streams.ActiveTransfer

// Added
public class ActiveTransfer implements Serializable // new internal class; an upload or download request currently being served for a UI
public ActiveTransfer(String path, Element owner) // owner must not be null
public void terminate() // makes the streams handed out for this transfer stop transferring bytes
public boolean isTerminated()
public VaadinRequest wrapRequest(VaadinRequest request) // returns the request as-is if it is not servlet based
public VaadinResponse wrapResponse(VaadinResponse response) // returns the response as-is if it is not servlet based
public String getDescription() // for logging

com.vaadin.flow.component.internal.UIInternals

// Added
public Registration registerActiveTransfer(ActiveTransfer transfer) // the registration must be removed when the request has been served
public boolean hasActiveTransfers()
public void terminateActiveTransfers() // called when the session is invalidated

com.vaadin.flow.internal.streams.TerminableStreamsUtil is new but package-private, so it adds no API surface.

Test summary

#	Status	What the test verifies	Why it matters
1	✅	A closed UI stays in the session while a download for it runs, and is detached by the next cleanup after the request ends	The core fix; if it regressed, callbacks would break again, or a closed UI would leak in the session
2	✅	An upload whose UI is closed mid-transfer still delivers the full content to the completion callback	The main reported symptom: a finished upload that was silently dropped
3	✅	Invalidating the session during a download served through `TransferUtil` throws `IOException` and leaves the response truncated	Without it, a transfer keeps sending data after the session is gone
4	✅	Invalidating the session during a custom `ElementRequestHandler` that writes straight to the response stops the next write; only the bytes written before it arrive	Proves the block works for any handler, not just the built-in ones
5	✅	After `terminate()`, the response output stream and writer, and the request input stream, reader and multipart part stream all refuse to transfer more	Each stream is a separate leak path out of a dead session
6	✅	The wrappers delegate everything that is not content: part metadata/headers/`write`/`delete`, servlet stream `isReady`/`isFinished`/`available`/listeners/`close`	A broken delegation would break normal uploads and downloads
7	✅	A non-servlet request/response is returned unwrapped	Keeps non-servlet environments working instead of failing at a bad cast
8	❗ gap	End-to-end: an ongoing upload is terminated when the session is invalidated	Covered only at unit level (row 5); no request-handler-level test like rows 3–4
9	❗ gap	Registering and terminating transfers from several threads at once	The transfer set is intentionally outside the session lock, so concurrency is the risky part

StreamRequestHandlerTest.ongoingDownload_uiClosed_uiStaysAttachedUntilDownloadHasCompleted → 1
StreamRequestHandlerTest.ongoingUpload_uiClosed_uploadCompletionCallbackIsInvoked → 2
StreamRequestHandlerTest.ongoingDownload_sessionInvalidated_downloadIsTerminated → 3
StreamRequestHandlerTest.ongoingDownload_customHandlerWritingToResponse_sessionInvalidatedTerminatesIt → 4
ActiveTransferTest.outputStream_terminated_refusesToWriteMore, .writer_terminated_refusesToWriteMore, .inputStream_terminated_refusesToReadMore, .reader_terminated_refusesToReadMore, .parts_terminated_partStreamRefusesToReadMore → 5
ActiveTransferTest.part_everythingButTheContent_delegated, .servletStreams_everythingButTheContent_delegated → 6
ActiveTransferTest.nonServletRequestAndResponse_leftUnwrapped → 7
ActiveTransferTest.description_containsPathAndOwner → no essential row; it only pins the wording of the log message

Deliberately untested: the one-per-type warning for an unwrappable request or response (logging only), and the new Javadoc. The tests that were in ActiveTransferLifecycleTest moved into StreamRequestHandlerTest; that file is deleted, not lost.

Adds failing tests for the behavior described in #25409: - a UI that is closed while a download for it is being served stays attached to the session until the download has completed, and is detached once it has; - the completion callback of UploadHandler.toTempFile is invoked even when the browser tab is closed in the middle of the upload; - invalidating the session terminates an ongoing download instead of letting it run to the end. All three currently fail: the closed UI is detached at the end of the request that noticed the close, which makes the upload callback fail with a UIDetachedException, and an ongoing transfer is unaffected by session invalidation.

An ElementRequestHandler implemented directly, without going through TransferUtil, can sit blocked waiting for its data source. Nothing terminates it today when the session is invalidated, so the test fails by running to completion.

A handler that writes to the VaadinResponse output stream instead of going through TransferUtil keeps writing after the session has been invalidated.

…ation Ongoing upload and download requests are now tracked per UI, which addresses two unmet expectations: A UI that is closed, e.g. by closing the browser tab, while a transfer for it is being served is kept attached to its session until that request has been served. Transfer progress listeners and the completion callbacks of handlers such as UploadHandler.toTempFile are bound to that UI, so they were previously ineffective or failed with a UIDetachedException. The UI is detached by the regular end-of-request cleanup of the request that serves the last transfer. Invalidating the session terminates the transfers that are ongoing for its UIs and logs a warning with the path, the owner component and the number of bytes transferred so far for each of them. Termination is enforced by wrapping the request and the response passed to the handler, so that every stream the framework hands out - the response output stream and writer, the request input stream and reader, and the streams of multipart parts - refuses to transfer more bytes and throws an IOException instead. This covers the built-in handlers as well as a handler that implements ElementRequestHandler directly, since all data that reaches the client passes through those streams. A handler that is blocked waiting for its own data source is not interrupted, but it can no longer deliver anything once it resumes. A transfer progress notification for a UI that has been detached is now skipped instead of throwing a UIDetachedException, since that only happens when the session has been invalidated and the transfer is thus being terminated anyway. That keeps the failure the application sees the one that actually terminated the transfer. Fixes #25409

github-actions · 2026-09-02T06:41:47Z

Test Results

1 475 files + 1 1 559 suites +1 1h 42m 2s ⏱️ + 4m 7s
12 252 tests +13 12 184 ✅ +13 68 💤 ±0 0 ❌ ±0
12 570 runs +13 12 502 ✅ +13 68 💤 ±0 0 ❌ ±0

Results for commit 9d8ad81. ± Comparison against base commit a122814.

♻️ This comment has been updated with latest results.

The Sonar quality gate flagged the new code for low coverage and for duplication. The two input stream wrappers had the same reading logic, so the servlet one now reads through the plain one instead of repeating it. The defensive branches for an unparseable UI id and for a missing transfer owner are gone as well: the UI id always comes from the resource URI that the request was matched against, and the owner element is required by the transfer. ActiveTransferTest covers what the wrapped request and response hand out, and the servlet stream helpers that it shares with ActiveTransferLifecycleTest now live in a test utility.

The counter only ever appeared in the warning that is logged when a transfer is terminated, and it lumped together the bytes read from the request and the bytes written to the response. The streams now only check whether the transfer has been terminated, and the warning identifies the transfer by its path and owner.

Addresses review feedback: - the terminating streams extend FilterInputStream, FilterWriter and FilterReader instead of repeating the plain delegations; - a transfer describes its owner through the same StateNode logic that other framework messages use, which also brings in the create and attach locations in development mode; - terminating the transfers of a UI is a UIInternals concern rather than a VaadinService one, and registering one uses Registration.addAndRemove; - the field holding the transfers documents why it is not protected by the session lock; - the resource path is parsed once per request; - a request or response that cannot be wrapped is now reported with a warning, once per JVM, instead of silently serving transfers that cannot be terminated; - UploadHandler documents the lifecycle of an ongoing upload the same way DownloadHandler documents a download.

Basing the terminating streams on FilterInputStream and FilterReader made skip bypass the termination check, since those classes pass it straight to the wrapped stream instead of reading through the methods that are checked. A terminated upload could thus still consume the request body through the stream of a multipart part. Terminating the transfers of a UI now also terminates all of them before describing any of them for the log, and describing a transfer tolerates a failure in rendering the owner component, so that application code cannot leave the remaining transfers running.

…nsfers-per-ui

Addresses review feedback: - a transfer is described through the existing public StateNode.describe(), which keeps the description logic in one place and never runs application code, so neither the method nor its caller needs to guard against a failing toString(); - the streams no longer check whether the transfer has been terminated in methods that expose no data of their own, which are skipping and flushing; - an unwrappable request or response is reported once per type rather than once per JVM, and the message tells what will not happen if the session is invalidated rather than implying that it already has.

The tests that cover what happens to a UI and its session while a transfer is being served now live in the test class of the request handler that serves those requests, instead of in a test class of their own. They keep their own request, response and UI, since they need the servlet types and a UI that is added to the session, whereas the existing tests there use mocks and an uninitialized UI. Reaching the end-of-request session cleanup from another package is done through a mock service method, and the Mockito calls of the tests added by this branch use static imports.

The UI, request and response that one of these tests serves a transfer with are collected into a record that the setup helper returns, instead of living in fields that the rest of the class has no use for. The upload test keeps its content in memory rather than writing a temporary file, and the test for a handler that writes to the response directly now writes a single chunk after the invalidation, since the first write after it already has to fail.

Artur- · 2026-09-14T10:00:09Z

@claude does this make sense?

claude · 2026-09-14T10:00:34Z

Claude finished @Artur-'s task in 1m 49s —— View job

Does this make sense? — yes, the approach is sound

I traced the full lifecycle (StreamRequestHandler → UIInternals → VaadinService) and the design holds together. Summary of the mechanism, to confirm I read it the way you intended:

Keeping a closed UI alive

callElementResourceHandler creates an ActiveTransfer, and while holding the session lock looks up the UI and registers the transfer (StreamRequestHandler.java:209-213). The handler then runs outside the lock with wrapped request/response, and the finally removes the registration (:222-225).
removeClosedUIs now skips a closing UI that hasActiveTransfers() (VaadinService.java:1642-1652), and cleanupSession runs at the end of every request (:1602-1605). So the stream request that serves the last transfer removes its own registration, then its own end-of-request cleanup detaches the UI. No extra trigger needed, no permanent leak. ✔️

Terminating on invalidation

fireSessionDestroy calls terminateActiveTransfers() per UI under session.access (VaadinService.java:955), which flips a volatile flag; the wrapped streams then throw IOException on the next byte. ✔️
The lock discipline is the key correctness argument and it checks out: registration and termination are serialized by the session lock, so the ordering is either "register → later terminate" or "UI already detached → owner not attached → request rejected" (the comment at :199-208). The concurrent set only exists for the request thread removing its own registration off-lock, which the field comment at UIInternals.java:318-325 now documents. ✔️

Progress notifications

notifyThroughUI swallowing UIDetachedException at debug (TransferProgressAwareHandler.java:428-436) is the right call — the real cause surfaces as the transfer's own IOException instead of a detached-UI error.

A few things worth a second look (none blocking; Legioth already approved)

Unbounded retention of a closed UI by a stalled transfer. A closed UI with a hung/very slow transfer stays pinned to the session (and keeps its component tree alive) until the transfer ends or the session times out. That's by design and bounded by session timeout, but it does mean a slow client can keep a "closed" UI resident longer than before. Probably worth one sentence in the UI.close() / handler Javadoc so it isn't a surprise.
The two self-declared test gaps are the real risk areas. Row 8 (end-to-end upload termination on invalidation, currently only unit-level in row 5) and row 9 (concurrent register/terminate). The concurrency path is exactly the part that deviates from the session-lock norm, so a small multi-threaded test on the concurrent set would earn its keep even though reasoning says it's safe.
ui == null window. If getUIById returns null (UI already gone as the session dies), the handler still runs with wrapped streams but transferRegistration is null, so terminate() can never be called on it — that transfer would run to completion. It's a narrow race and the session is dying anyway, but it's a case where "invalidation kills the transfer" quietly doesn't hold. Fine to leave, just flagging it's there.
Minor: terminateActiveTransfers iterates List.copyOf(activeTransfers) twice (terminate, then log). Intentional per your comment (UIInternals.java:2080-2089) so a description failure can't leave others running — makes sense, no change needed.

Bottom line: the model — track per-UI, defer detach while a transfer is in flight, terminate the handed-out streams on invalidation, wrap servlet types so casts keep working and pass non-servlet through — is coherent and the locking is correct. It makes sense. 👍
• branch feat/track-active-transfers-per-ui

sonarqubecloud · 2026-09-15T08:12:19Z

Quality Gate passed

Issues
14 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Explain that a closed UI stays attached until its ongoing upload or download finishes, and that invalidating the session instead terminates that transfer, on the application lifecycle, uploads, and downloads pages. Documents vaadin/flow#25431 (`9d8ad814fa8c97d9ef20415e360695c875b3d28d`).

github-actions · 2026-09-18T13:09:43Z

Pull request created: #6082

Generated by Documentation Bot · agent · 103.8 AIC · ⌖ 6.17 AIC · ⊞ 11.2K

github-actions · 2026-09-18T13:09:45Z

Documentation Bot: Draft documentation pull request for this change: vaadin/docsvaadin/docs#6082

Files updated:

articles/flow/advanced/application-lifecycle.adoc
articles/flow/advanced/upload-resources.adoc
articles/flow/advanced/downloads.adoc

It was written from the state of this pull request as you see it now. Please review it and mark it ready for review.

Generated by Documentation Bot for #25431 · agent · 103.8 AIC · ⌖ 6.17 AIC · ⊞ 11.2K · ◷

) Documentation for vaadin/flow#25431 by `@totally-not-ai`[bot]. > [!NOTE] > The source pull request is merged, so this documentation describes the final > shape of the change. Please review it and mark it ready for review. **Change categories:** BEHAVIOR_CHANGE | File | Change | |------|--------| | `articles/flow/advanced/application-lifecycle.adoc` | Note that a UI serving an ongoing upload or download stays attached to its session until the transfer is handled, and that invalidating the session instead terminates any ongoing transfer. | | `articles/flow/advanced/upload-resources.adoc` | Explain that an ongoing upload keeps its UI attached across a close/tab-close so its callback still runs, but is terminated and discarded if the session is invalidated. | | `articles/flow/advanced/downloads.adoc` | Explain the same lifecycle for download progress listeners: they keep firing for a download whose UI has closed, but are notified through `onError` instead of `onComplete` if the session ends first. | Auto-generated by the Documentation Bot — review before merging. > Generated by [Documentation Bot](https://github.com/vaadin/flow/actions/runs/35347816994) for #25431 · agent · 103.8 AIC · ⌖ 6.17 AIC · ⊞ 11.2K · [◷](https://github.com/search?q=repo%3Avaadin%2Fdocs+%22gh-aw-workflow-id%3A+doc-bot%22&type=pullrequests) > - [x] expires  on Oct 18, 2026, 1:09 PM UTC

vaadin-bot · 2026-09-29T13:51:27Z

This ticket/PR has been released with Vaadin 25.4.0-alpha1.

totally-not-ai Bot added 4 commits September 1, 2026 09:02

test: capture terminating a handler writing straight to the response

2a7d3c4

A handler that writes to the VaadinResponse output stream instead of going through TransferUtil keeps writing after the session has been invalidated.

github-actions Bot added the +0.1.0 label Sep 2, 2026

totally-not-ai Bot added 2 commits September 2, 2026 06:49

Legioth requested changes Sep 2, 2026

View reviewed changes

totally-not-ai Bot added 3 commits September 2, 2026 07:46

Merge remote-tracking branch 'origin/main' into feat/track-active-tra…

79f2187

…nsfers-per-ui

Legioth requested changes Sep 3, 2026

View reviewed changes

totally-not-ai Bot and others added 2 commits September 3, 2026 11:39

Merge branch 'main' into feat/track-active-transfers-per-ui

8a737a1

Legioth requested changes Sep 8, 2026

View reviewed changes

Comment thread flow-server/src/test/java/com/vaadin/flow/server/ActiveTransferLifecycleTest.java Outdated

Comment thread flow-server/src/test/java/com/vaadin/flow/internal/streams/ActiveTransferTest.java Outdated

Legioth self-assigned this Sep 9, 2026

Legioth requested changes Sep 14, 2026

View reviewed changes

Legioth approved these changes Sep 14, 2026

View reviewed changes

Artur- added 2 commits September 15, 2026 09:25

Merge branch 'main' into feat/track-active-transfers-per-ui

8f8bdf2

Merge branch 'main' into feat/track-active-transfers-per-ui

9d8ad81

mshabarov added this pull request to the merge queue Sep 18, 2026

mshabarov added this to Vaadin Flow | Hilla | Kits ongoing work Sep 18, 2026

mshabarov moved this to 🔎Iteration reviews in Vaadin Flow | Hilla | Kits ongoing work Sep 18, 2026

Merged via the queue into main with commit 487fb8c Sep 18, 2026
41 checks passed

mshabarov deleted the feat/track-active-transfers-per-ui branch September 18, 2026 13:01

github-project-automation Bot moved this from 🔎Iteration reviews to Done in Vaadin Flow | Hilla | Kits ongoing work Sep 18, 2026

vaadin-bot mentioned this pull request Sep 18, 2026

docs: describe transfer lifecycle across UI close and session end vaadin/docs#6082

Merged

vaadin-bot added the Released with Vaadin 25.4.0-alpha1 label Sep 29, 2026

Conversation

totally-not-ai Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Use case

API Changes

com.vaadin.flow.internal.streams.ActiveTransfer

com.vaadin.flow.component.internal.UIInternals

Test summary

Uh oh!

github-actions Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Artur- commented Sep 14, 2026

Uh oh!

claude Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Does this make sense? — yes, the approach is sound

A few things worth a second look (none blocking; Legioth already approved)

Uh oh!

sonarqubecloud Bot commented Sep 15, 2026

Quality Gate passed

Uh oh!

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

vaadin-bot commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

totally-not-ai Bot commented Sep 2, 2026 •

edited

Loading

github-actions Bot commented Sep 2, 2026 •

edited

Loading

claude Bot commented Sep 14, 2026 •

edited

Loading