Fix Writer Write-after-Close, ReadFrom position and pipe handling, and unreleased resources

Aperta
  • Qualcosa non va?
#181 2 commenti 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@ffumero2003 ci sta già lavorando.

Dal 28/10/2026.

  • #281 di @sophiecarreras — aperta

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

bug

Context

b2.Writer's io.Writer, io.ReaderFrom and io.Closer behavior has several defects:

  1. Write after Close. closed is set only on the large-file success path. After a small file's Close, Write dereferences the released pooled buffer and panics. After Close of a never-written writer, Write succeeds and, past ChunkSize, starts a large file nothing will finish.
  2. io.Copy from a pipe uploads an empty object. ReadFrom treats any io.ReadSeeker as seekable. *os.File always has Seek, so io.Copy(w, os.Stdin) with piped stdin fails with "illegal seek" instead of buffering, and Close then writes a zero-byte object.
  3. ReadFrom ignores the reader's position. Without Resume, it uploads from offset 0 and returns the full size even when the source was already advanced.
  4. Unreleased resources. A writer closed without any Write stays in Client.sWriters with its buffer open (an empty temp file with UseFileBuffer); two worker error paths skip cnk.buf.Close(); NewWriter's context is never cancelled on success; ReadFrom's buffered paths use copyContext without a closer, so the copy goroutine keeps reading after ReadFrom returns on cancellation.
Current behavior

At a5e0d13:

  • b2/writer.go:282-287 and :769: closed set only after a large-file finish.
  • b2/writer.go:723-727: the never-written path returns before removeWriter and buffer Close are deferred (:728-736); b2/buffer.go:145-155 and :127-131.
  • b2/writer.go:649-715: ReadFrom checks only for io.ReadSeeker (:650), seeks to the end (:671) and starts at 0 (:682).
  • b2/writer.go:201-204 and :214-217: worker returns without closing the chunk buffer (the upload-error path at :222-226 does close it).
  • b2/b2.go:738 and b2/writer.go:152: the writer context is cancelled only on error.
  • b2/writer.go:652 and :668: copyContext (b2/reader.go:378-380) passes no closer to copyContextWithCloser (b2/reader.go:384-405). The sync engine (Backblaze/blazer#135) reaches this path when it wraps the source in an io.TeeReader (b2/sync/sync.go:399-412).

Reproduced locally (test server, -race):

Case Result
small Write + Close, then Write panic: nil pointer dereference
Close without Write, then Write past ChunkSize no error, large file started
io.Copy(w, pipe) seek: illegal seek; Close returns nil, empty object stored
file at offset 8 of HEADER--payload, ReadFrom object starts with HEADER--
50 empty + 50 small writers closed 50 entries left in Client.sWriters
empty writer with UseFileBuffer 1 temp file left
Expected behavior

Write and ReadFrom return ErrClosed after Close; pipes upload their data; ReadFrom starts at the current position; every writer releases its resources.

Acceptance criteria

  • R1. Write after Close returns ErrClosed on small, empty and large paths, with no panic and no new upload.
  • R2. io.Copy(w, r) where r is an *os.File pipe uploads the piped bytes.
  • R3. ReadFrom on a seeker at offset k uploads from k and returns size - k.
  • R4. After closing N empty writers, Client.Status().Writers and FileBufferDir are empty.

In scope

Proposed change
  1. Set closed = true at the start of Close on every path.
  2. In ReadFrom, start at Seek(0, io.SeekCurrent) and fall back to buffering when seeking fails; return bytes actually sent.
  3. Defer removeWriter and buffer Close before the never-written branch; close chunk buffers on every worker exit.
  4. Call w.cancel() at the end of Close.
  5. In ReadFrom, use copyContextWithCloser with a closer when available, or wait for the copy goroutine, or document the behavior.

Out of scope

  • Unrelated work outside the components and behavior described in In scope.

Verification

Run the repository regression checks with live credentials unset:

unset B2_APPLICATION_KEY_ID B2_APPLICATION_KEY B2_ACCOUNT_ID B2_ACCOUNT_KEY
go build ./...
go vet ./...
go test -race -count=1 ./b2/...
  • Run the supplied small/empty/large Close cases, OS-pipe copy, positioned-reader case, and resource-cleanup checks. Assert errors, content, byte counts, registries, and temp files as specified.
  • Demonstrate failing regressions before each fix and passing results afterward; include concurrent/error/cancellation paths in the race run.
  • Expected result: the checks above succeed and the observable outcomes in Acceptance criteria are demonstrated. If a check cannot run, report Could not run because: ...; if it starts but fails, report Could not complete because: .... A skipped check is not a pass.

Links

Open Backblaze/blazer#54 is a different panic in Close. Merged Backblaze/blazer#131 added copyContextWithCloser for downloads. Closed Backblaze/blazer#56 and Backblaze/blazer#58 fixed earlier worker issues. Merged Backblaze/blazer#135 added a ReadFrom caller in b2/sync.

Lingua principale
Go
Stelle
66
Fork
18
Merge medio
1g 10h
PR unite (30g)
132

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Backblaze/blazer

Tutte le issue di Backblaze/blazer

Issue simili

Altre issue su Go

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.