fix(tcpclient, iostream): close SSLIOStream on TLS handshake timeout to prevent fd leak

[armorbreak001]

Problem (tornado#3614)

When TCPClient.connect() is called with both ssl_options and timeout, a TLS handshake timeout causes permanent file descriptor leak.

Root Cause

start_tls() transfers socket ownership in three steps:

1. Extract raw socket from original IOStream → self.socket = None
2. Wrap in SSL → create new SSLIOStream (local variable only)
3. Return Future[SSLIOStream] that resolves when handshake completes

When gen.with_timeout() fires:

• Caller gets TimeoutError
• Original stream's .socket is already None → close() is no-op
• The SSLIOStream (holding the real fd) is only reachable through the inner Future
• gen.with_timeout explicitly does not cancel the wrapped Future
• SSLIOStream stays on IOLoop forever → fd leaked permanently

Impact

Every timed-out TLS connection leaks one file descriptor. Long-running services that retry connections (e.g. HTTP clients with short timeouts) will eventually hit the OS fd limit (EMFILE / "too many open files").

Fix

Two files, minimal change:

tornado/iostream.py

In start_tls(): store the SSLIOStream as self._tls_stream before returning the future.

self._tls_stream = ssl_stream  # Keep reference for timeout cleanup

tornado/tcpclient.py

In connect(): on TimeoutError during start_tls(), close _tls_stream.

except gen.TimeoutError:
    tls_stream = getattr(stream, "_tls_stream", None)
    if tls_stream is not None:
        tls_stream.close()
        stream._tls_stream = None
    raise

Testing

New test: TestTLSHandshakeTimeoutCleanup.test_tls_handshake_timeout_closes_stream

• Opens TCP server socket (no TLS accept)
• Calls TCPClient.connect() with SSL + 0.01s timeout
• Expects TimeoutError
• Verifies no fd leaked via /proc/self/fd count

All existing tests pass:

• tcpclient_test.py: ✅ 27 passed, 4 skipped
• iostream_test.py: ✅ 142 passed, 61 skipped

Fixes #3614

[bdarnell]

This is intended to replace #3615, right? Please update existing PRs instead of starting new ones. And make sure that at least the checks run in the "quick" ci config pass before pushing changes.

[bdarnell]

This feels dirty - we don't cancel the coroutine, so it could wake back up and find that its internal state (i.e. self._tls_stream) has been modified.

I think in this case it's all more or less harmless (except for the logging that is currently failing CI) because we don't rely on self._tls_stream after setting it and closing the socket will cause it to generate an internal StreamClosedError, but it's difficult to reason about.

I think it would probably be better to push awareness of the timeout into start_tls() so that we don't handle the timeout at a different level from other errors. Alternately, cancelling the task might do the job more cleanly for us (or at least it's supposed to. Tornado was originally built with non-cancelable futures and I'm not sure how much cancellation works now that we've picked that feature up from asyncio).

[bdarnell]

This is platform-specific and at least needs to be guarded.

Is counting open FDs the best way to do this? This might be a time when mocks would be most appropriate, but that seems messy too.

[bdarnell]

Why?

[armorbreak001]

Closing in favor of #3615, which has been updated with the improved approach (tls_future.cancel() + _tls_stream cleanup + platform guard for test). Thanks for the review @bdarnell!