Prevent orphaned TCP socket

[mueslo]

When an exception occurs during the TLS layer creation, the created TCP socket is not connected to any connection and thus never closed leading to a TCP socket leak, see home-assistant/core#60150

[florimondmanca]

Hi! This repository is in the dying end — we've now moved a refined version of the core networking code to httpx as of HTTPX 0.21.0, which improved connection pooling robustness a lot (including connection leaks).

Are you still encountering this issue while running against HTTPX 0.21.0?

[mueslo]

@florimondmanca Yes, the issue occurred with the latest versions of httpx 0.21.1 +httpcore 0.14.3 (see linked homeassistant thread). And my PR prevents the observed TCP socket leak.

[lovelydinosaur]

Thanks, good work in tracking this down!

Wonder if we ought to resolve this at the backend level instead. (So that the start_tls() method is responsible for closing the socket if it fails.)

[mueslo]

Thanks for the response!

Yes, I thought about that too, but since 1) it would need to be added to each backend separately and since 2) perhaps in some cases you might want to retry establishing a TLS layer on the same TCP connection I went with the simplest choice of just catching the exception in the uppermost layer possible.

A feasible alternative I think could be to add the TCP stream as a member of the AsyncHTTPConnection class so it can be correctly removed if it exists when the connection is closed/removed, but since the TCP stream is not really used anywhere outside the AsyncHTTPConnection._connect method I thought it'd be unnecessary complexity.

[elupus]

We have confirmation that this patch indeed solves our issues. home-assistant/core#57093 (comment)

[mueslo]

@tomchristie any chance of getting this (or an equivalent) fix merged before early/mid January so the fix can be shipped in HomeAssistant 2022.2?

[elupus]

@mueslo can you rebase this?

[nikrays]

My desire for the new year was to fix this problem, now another update has arrived, but I see that the problem has not been fixed, what is fashionable to do?🤦🏻

[lovelydinosaur]

I've addressed this in #475