| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I do not like this.
I have not tested the code on Windows yet, very likely more try/exept or return code checks should be added in Windows specific code. |
Sorry, something went wrong.
| _winapi.PeekNamedPipe(self._handle)[0] != 0): | ||
| return True | ||
| except OSError as err: | ||
| if err.errno == errno.EBADF: |
There was a problem hiding this comment.
We should not get EBADF. We should just not call functions on closed handle. If we land in this case, we failed badly before, no?
Sorry, something went wrong.
There was a problem hiding this comment.
How can we know that it was closed in other thread without using locks?
You can get EBADF in the following scenario:
It is the bast case. The worst case is:
Sorry, something went wrong.
| raise | ||
| except TypeError: | ||
| self._check_closed() | ||
| raise |
There was a problem hiding this comment.
Would it be possible to split this change in two parts? Keep this change for _check_closed() change which now raises BrokenPipeError, and you changes around close(): these ones look straightforward are correct. But write a separated PR to add these try/except?
I dislike these try/except. If there is a risk to land into EBADF case with the current code, maybe some kind of locking is needed to not call the Windows API with a handle, while another thread call close() which invalidates the handle.
Sorry, something went wrong.
|
The more I look at this, the more I dislike it. I do not see how is it possible to safely close a file descriptor on other thread without locking. Making Connection and PipeConnection truly thread-safe in a task on completely different level. I opened #109780 instead. |
Sorry, something went wrong.
Maybe we should just give up and document that they are not thread-safe. I recall that I did that once on some asyncio classes. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.