Prevent concurrent AIOFile.open() calls re-opening the file #69
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AIOFile.open() intended to raise an error if it was called after the _file_obj had been closed. However in practice it could never happen, as the function would always return None from the first if statement. This is a change to the behaviour of open(), but it does appear to be the intended behaviour, and it won't change the overall behaviour of AIOFile, as even if someone was relying on open() succeeding after close(), all methods that operate on the _file_obj would fail after the re-open.
This test ensures open() only opens an underlying file once when called concurrently. This test currently fails, as described in: mosquito#68
AIOFile.open() now uses a lock to prevent concurrent calls opening a backing file multiple times. This fixes mosquito#68
An alternate implementation to prevent concurrent open() calls opening a backing file multiple times. This implementation uses the Future returned by _run_in_thread() to synchronise concurrent open calls. This fixes mosquito#68
|
released in 3.8.0 |
|
Great, thanks for merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I implemented this with an
asyncio.Lockas you suggested ( f6b5b46 ), but then I noticed that_run_in_thread(open, ...)that provides the file obj returns anasyncio.Future, so we can use that to synchronise callers without needing an explicit lock. Either seems fine to me.Also, I noticed a related minor bug while doing this. It seems the intention of the code was that calling
open()after a previousclose()would raise anInvalidStateError, but in fact this could never happen, asopen()would always return from the first if statement before check to raise the error would happen.This fixes #68.