Skip to content

fix: return an error instead of panicking when a BufWriter is reused after an error#816

Open
hindriix wants to merge 1 commit into
apache:mainfrom
hindriix:fix/bufwriter-panic-after-error
Open

fix: return an error instead of panicking when a BufWriter is reused after an error#816
hindriix wants to merge 1 commit into
apache:mainfrom
hindriix:fix/bufwriter-panic-after-error

Conversation

@hindriix

Copy link
Copy Markdown

Which issue does this PR close?

Closes #810.

Rationale for this change

BufWriter panics with `async fn` resumed after completion when it is
used again after an internal operation has failed.

When the future backing BufWriterState::Prepare (multipart upload
initialisation via put_multipart_opts) or BufWriterState::Flush
(put_opts / upload.finish()) resolves to an error, the ? operator
propagates the error but leaves the now-completed future in self.state.
The next poll (poll_write, poll_flush, poll_shutdown, or the put
convenience method) re-enters that arm and re-polls the completed future,
which panics inside the generated async state machine:

thread '...' panicked at object_store/src/buffered.rs:407:71:
`async fn` resumed after completion

This is easy to hit in practice: e.g. building a parquet::arrow::AsyncArrowWriter
on top of a BufWriter, having a write fail, and then calling close()
(or writing again) turns a recoverable error into a panic. As #810 notes,
calling shutdown/close after an error should return an error, not panic.

What changes are included in this PR?

  • Add a terminal BufWriterState::Errored state.
  • When a backing future in Prepare/Flush resolves to an error, transition
    the writer into Errored (dropping the completed future) before returning
    the error, so it is never re-polled.
  • Handle Errored in put, abort, poll_write, poll_flush, and
    poll_shutdown by returning an error rather than panicking.
  • Add a regression test (test_buf_writer_reuse_after_error_returns_error)
    that reproduces the original panic and asserts an error is returned instead,
    both for shutdown-after-error and write-after-error.

The successful (non-error) code paths are unchanged.

Are there any user-facing changes?

Yes, but only for a previously-panicking path: a BufWriter whose internal
upload has failed now returns an error from subsequent write/flush/
shutdown/put/abort calls instead of panicking. There are no public API
or signature changes.

…after an error

When a future backing `BufWriterState::Prepare` or `Flush` resolved to an
error, the `?` operator propagated the error but left the now-completed
future in `self.state`. Polling the writer again (via `poll_write`,
`poll_flush`, `poll_shutdown` or `put`) re-polled that completed future,
panicking with "`async fn` resumed after completion".

Transition to a new terminal `BufWriterState::Errored` state when a backing
future fails, so subsequent operations surface an error instead of panicking.

Closes apache#810
@hindriix hindriix mentioned this pull request Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BufWriter panic

1 participant