Skip to content

Cancelling an in-progress chunked upload permanently leaks its concurrency slots, deadlocking the chunk queue #339

Description

@smarcet

Summary

Aborting an XHR fires neither onload nor onerror, so onChunkComplete() never runs for it. The removedfile handler aborts every in-flight XHR of the removed file without releasing the concurrency slots those chunks took, and chunksInFlight is never reset outside the constructor and componentWillUnmount. Because a large chunked upload keeps the queue saturated, cancelling one leaves chunksInFlight === maxConcurrentChunks permanently, and processChunkQueue() stops dispatching for the lifetime of the mounted component.

Net effect: after a user cancels one in-progress upload, every subsequent upload in that component silently never starts — no XHR is sent, no error event is emitted, the progress bar sits at 0.

This is independent of the maxConcurrentChunks value: the queue is saturated at whatever the ceiling is, so the leak equals the ceiling either way.

Steps to reproduce

  1. Mount UploadInputV3 with parallelChunkUploads and a djsConfig.chunkSize small enough that the test file yields more chunks than maxConcurrentChunks (e.g. 60 MB at 5 MB chunks with the default ceiling of 6).
  2. Drop the file and wait until the queue saturates.
  3. While the row still reads Loading, click the trash icon on that row.
  4. Without unmounting the component, drop another file.

Expected: the second file uploads.
Actual: it is queued and never dispatched. Nothing is sent, nothing errors.

Assertable directly on the instance after step 3: chunksInFlight === maxConcurrentChunks while chunkQueue.length > 0.

Root cause

All references are main @ 5.0.56.

src/components/inputs/dropzone/index.js

  • chunksInFlight is incremented at dispatch (L61) and decremented in exactly one place, onChunkComplete() (L83), reached only from the xhr.onload (L451) and xhr.onerror (L494) wrappers.
  • It is reset to 0 in exactly two places: the constructor (L24) and componentWillUnmount (L239).
  • The removedfile handler (L368) aborts every non-DONE XHR for the file (L380–L384) and neither decrements chunksInFlight nor purges chunkQueue of that file's pending entries.

src/components/inputs/upload-input-v3/index.js

  • The in-progress row (uploadingFiles.map, L356) renders the delete IconButton (L398–L401) unconditionally, unlike the success icon which is gated on file.complete. So the control is reachable precisely while chunks are in flight.
  • handleDeleteUploading (L233) sets dzFile._userCanceled = true and calls removeFile(dzFile) (L244–L245), which routes through Dropzone's cancelUploadxhr.abort()emit('removedfile').

Not covered by the open PRs

Checked main, #330 and #334: in all three, chunksInFlight appears only at the increment, the single decrement, and the two resets. #330 fixes a different leak on the same counter (xhr.ontimeout not releasing its slot) by wrapping ontimeout; abort remains uncovered because it fires none of the three handlers.

Suggested fix

In the removedfile handler, release the slots the aborted chunks were holding and drop their queued siblings, then re-drive the queue:

const xhrs = this.activeXHRs.get(file);
if (xhrs) {
    xhrs.forEach(xhr => {
        if (xhr.readyState !== XMLHttpRequest.DONE) {
            // abort() fires neither onload nor onerror, so the slot this chunk
            // took has to be released here or it is lost for good.
            if (xhr._tookChunkSlot) {
                this.chunksInFlight = Math.max(0, this.chunksInFlight - 1);
            }
            xhr.abort();
        }
    });
    this.activeXHRs.delete(file);
}

// A removed file's pending chunks must not be dispatched later.
this.chunkQueue = this.chunkQueue.filter(({ files }) => !files.includes(file));

this.processChunkQueue();

with xhr._tookChunkSlot = file._isThrottledChunk set in the sending handler, reusing the per-file flag #330 introduces. Tagging the XHR rather than reading the file flag at abort time keeps it correct when only some of a file's requests went through the queue.

A regression test in the shape of #330's test_dropzone_ontimeout_releases_chunk_slot would cover it: seed chunksInFlight, run the sending handler, emit removedfile, assert the counter returned to 0 and the queue was purged.

Environment

openstack-uicore-foundation main @ 5.0.56; also verified by inspection on the published 5.0.49 bundle.

Found while reviewing a downstream consumer. Across the two repos that consume upload-input-v3 there are four call sites, and all of them can hit it — cancelling and retrying without unmounting deadlocks the queue in every case. Three of them happen to have an escape hatch, since they render the input inside a MUI Dialog with no keepMounted, so closing it unmounts the subtree and componentWillUnmount resets the counter: sponsor-services/src/components/UploadBtn/UploadDialog.js, summit-admin/src/components/upload-dialog/index.js and summit-admin/src/pages/companies/components/company-dialog.js.

The fourth has no such escape: summit-admin/src/pages/events/edit-event-material-page.js renders EventMaterialForm inline on a routed page (src/layouts/event-id-layout.js), and that form holds two UploadInputV3 instances. They stay mounted for the whole visit, so once a cancel leaks the slots the only recovery is navigating away or reloading.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions