Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: make Writer.CloseWithError block until writer exits #4168

Closed
wants to merge 1 commit into from

Conversation

dt
Copy link

@dt dt commented May 25, 2021

Writer.Close blocks on w.donec to return only after the spawned writer goroutine
has exited. Previously CloseWithError did not block on this channel however, meaning
it could return while the writer goroutine was still running, which could then lead
to a caller e.g. closing the underlying Client, leading to a crash in the still-running
goroutine.

This adds the same block on the channel to CloseWithError to ensure that however a
caller closes a Writer, it is fully closed when the close method returns and that
caller can safely Close the client.

Fixes #4167.

@dt dt requested a review from a team May 25, 2021 00:37
@dt dt requested a review from a team as a code owner May 25, 2021 00:37
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 25, 2021
@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label May 25, 2021
Writer.Close blocks on w.donec to return only after the spawned writer goroutine
has exited. Previously CloseWithError did not block on this channel however, meaning
it could return while the writer goroutine was still running, which could then lead
to a caller e.g. closing the underlying Client, leading to a crash in the still-running
goroutine.

This adds the same block on the channel to CloseWithError to ensure that however a
caller closes a Writer, it is fully closed when the close method returns and that
caller can safely Close the client.
@tritone tritone added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 26, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 26, 2021
@dt dt closed this Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storage: nil pointer crash in writer goroutine after CloseWithError
3 participants