Fix block upload worker leaks after an error

[ClaudiuSchuster]

Problem

uploadPendingBlocks returned as soon as it received the first block-upload error. The remaining goroutines could then block while sending to the unbuffered result channel, still holding their weighted-semaphore slots. Repeated retries progressively consumed all block-upload slots and left later uploads unable to start.

The acquisition-error path also continued to a deferred Release even though no slot had been acquired.

Fix

• Buffer one result per block.
• Receive every worker result before returning the first error.
• Return immediately when acquiring a slot fails.
• Add a focused regression test that repeats failing batches and then acquires the full semaphore capacity.
• Add an end-to-end regression test that drives the real uploadAndCollectBlockData path against a local HTTP server, fails one block and verifies that every permit is released.

The end-to-end test was contributed by Jose Vega in oss-singularity/Proton-API-Bridge#3.

Verification

Validated with Go 1.26 and golangci-lint 2.9.0, matching the repository workflow:

gofmt -d file_upload_batch_permits_test.go
golangci-lint run --timeout=180s
go test -v ./...
go test -v -race ./...

All commands pass. The repository's credential-gated Proton integration tests are skipped by their existing guard; all unit tests, including both regression tests, run successfully.

The fork's repository workflow also passes on the current head: Lint and Test.

Broader coordination

This pull request remains intentionally limited to the block-upload worker leak. Repository-wide backlog triage and OSS Singularity's offer of ongoing stewardship are documented in the rclone forum's existing Proton Drive x rclone discussion.

[ClaudiuSchuster]

Additional real-world validation from the downstream PDrive integration:

• the exact worker-drain change was deployed in a checksum-pinned rclone build during one 30.4 GiB VFS upload;
• six separated, same-process Proton block-upload 502 cycles occurred after that build became active;
• the same rclone process continued without a systemd restart and payload progress resumed after the failures;
• the upload has now completed with an empty queue and VFS cache.

This supports the semaphore-release behavior under repeated real backend failures, beyond the unit and race tests already listed in the PR.

A separate bounded retry for only the failed encrypted blocks is tracked in oss-singularity/proton-drive-linux#42 and was validated in oss-singularity/Proton-API-Bridge#2. I am deliberately keeping that follow-up out of this PR so this worker-lifecycle fix remains focused. Once this prerequisite lands, the retry can be proposed upstream as a single independent commit.

[jomplox]

Independent confirmation of this bug and fix from a different deployment.

We hit the same failure on rclone v1.74.4 syncing ~1.1M files: a 502 POST .../storage/blocks failed one block, the collector returned on the first error, the sibling goroutines stayed blocked on the unbuffered channel, and after enough failed batches the process sat in Acquire forever with no sockets open (futex wait, 0 B/s, systemd still activating).

We arrived at the same three changes (buffered channel sized to the batch, return after a failed Acquire, drain every result and return the first error) before finding this PR, and have been running them in production since 2026-09-01. One addition that may be useful: our regression test drives the real uploadAndCollectBlockData path against an httptest server that fails one block, with a semaphore of size 2, and asserts both permits are free afterwards. It fails with context deadline exceeded on master. Happy to push it onto this PR if wanted: jomplox@50b422c.

Related: the block body itself also needs to be replayable for the retry to succeed. The multipart reader is consumed by the first attempt, so a retried block after a connection drop sends an empty body. That half is in rclone/go-proton-api#9.

[ClaudiuSchuster]

Thank you, Jose — this is excellent independent confirmation and very useful coverage.

I reproduced the regression test from 50b422c against the exact bridge refs:

• master at 9d772d0, with the test only: failed 3/3 with context deadline exceeded;
• this PR at 1d2e00d, with the test only: passed 5/5 under go test -race;
• go test ./... and go vet ./... also passed on the PR head with that test.

It adds valuable coverage through the real uploadAndCollectBlockData path and complements the existing small repeated-batch test. Please do add it as a separate test-only commit if convenient; I would keep both tests. If pushing directly is awkward, I am happy to port it with attribution.

I also reproduced the two tests from rclone/go-proton-api#9: on master at 4e6ddcf, the connection-drop retry sent an empty multipart body and the 502 was not retried; on 4887ba0, both tests passed 5/5 under the race detector, and go vet ./... was clean. So that is a real, separate client-layer issue.

Our downstream bridge retry re-enters UploadBlock with a fresh reader and fresh signed link, while henrybear327#9 makes resty's retry of one UploadBlock call replayable. I agree that this should stay separate from #8; the planned bridge-level upstream follow-up should be reviewed against henrybear327#9 so the two layers do not accidentally create nested retry budgets.

Thanks again for the careful report and the production evidence.

[jomplox]

Thanks for reproducing it so thoroughly. Pushing straight onto an org fork branch is not possible from here, so the test is a one-commit, test-only PR against your branch: oss-singularity#3. It changes no code, just adds the test next to yours. Merge it into the branch or port it with attribution, whichever is easier for you.

[ClaudiuSchuster]

Done — thank you, Jose. I merged oss-singularity/Proton-API-Bridge#3 via rebase. Your test is now commit 47d69aa on this PR, with your authorship preserved.

The updated head passes the complete Go 1.26 workflow: golangci-lint, go test ./..., and go test -race ./... (run 33642216274). I also updated the PR description to document both complementary regression tests and credit the contribution.

This is exactly the additional end-to-end coverage we wanted. Much appreciated.