Wakeup the waiter with exception if handshake fails

[asvetlov]

During the work on python/cpython#17975 I've found a minor bug in uvloop: when SSL handshake fails with an exception the waiter is not waked up; it leads to hanging.

I have no idea how to write a test for it; in asyncio we had mocked test already for eof_received() during SSL handshaking. As I see EOF is the only scenario that can hang; there is a very low chance that a peer will call shutdown(sock, SHUT_WR) on own side without immediate close(sock) at least.

Anyway, please merge if you have no objections.

[fantix]

Thanks a lot for the PR! I'll try to add a test.

[1st1]

Awesome, Andrew, thank you.

[1st1]

@fantix Do you want to review Andrew's CPython PR python/cpython#17975?

[fantix]

@1st1 oh sure! I can look into that this weekend

[fantix]

@asvetlov Hi Andrew! I revisited this one today and tried to write a test - looks like in _fatal_error:

https://github.com/python/cpython/blob/7b3ab5921fa25ed8b97b6296f97c5c78aacf5447/Lib/asyncio/selector_events.py#L711-L721

It will call connection_lost(), which will eventually call _wakeup_waiter():

https://github.com/asvetlov/uvloop/blob/133ce7c81882958056da4a4b908e4704f8f0e31b/uvloop/sslproto.pyx#L324-L349

I tried to send EOF during handshake and couldn't reproduce the hanging situation - I've also tried to remove this _wakeup_waiter() call here in python/cpython#17975 and all asyncio-related tests passed. I'm worried that I might have missed something here, could you please shed some light?

[1st1]

@fantix I think we can pick up this PR and finish it, if @asvetlov isn't available. Or merge it as is if it's correct and add small touchups/tests in a follow up PR.

[fantix]

@1st1 I think we can merge this PR without further tests, because:

• I don't think this is a bug, because we are always closing the underlying transport, that will trigger SSLProtocol.connection_lost and wakeup the waiter.
• And the only case when this would hang is that the underlying transport didn't schedule a connection_lost() call on close. But in uvloop we hard-coded the transport to be one of TCPTransport, UnixTransport or SSLTransport, and the user would have to use Cython to create a buggy transport subclass to reproduce this issue. (I tried in CPython as a unit test but it seemed not quite possible)
• I think making an additional wakeup is harmless, and it may save some time by triggering the waiter early. Therefore, I think we could merge this.

[1st1]

@fantix You can go ahead and merge this yourself.