hindriix opened a new pull request, #816:
URL: https://github.com/apache/arrow-rs-object-store/pull/816
# 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:
```text
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
