gh-109370: Support closing Connection and PipeConnection from other thread

[serhiy-storchaka]

• Issue: Unexpected traceback output in test_concurrent_futures and crash #109370

[serhiy-storchaka]

I do not like this.

1. It is too complicated and verbose.
2. There is still a chance for race condition, if between pushing self.handler on the stack and calling the corresponding OS function, the handler was closed and a new file was open that reuses the same fileno.

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.

[vstinner]

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?

[serhiy-storchaka]

How can we know that it was closed in other thread without using locks?

You can get EBADF in the following scenario:

• Thread A reads self._handle and puts in on the stack.
• Thread B closes it.
• Thread A calls function with an already closed file descriptor on the stack.
• That function fails with EBADF errno.

It is the bast case. The worst case is:

• Thread A reads self._handle and puts in on the stack.
• Thread B closes it.
• Thread B or C opens a new file descriptor which reuses the same integer value.
• Thread A calls function with a file descriptor which now refers to unrelated file or socket.
• Unpredictable consequences.

[vstinner]

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.

[serhiy-storchaka]

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.

[vstinner]

Making Connection and PipeConnection truly thread-safe in a task on completely different level.

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.