Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tornado/iostream.py
Original file line number Diff line number Diff line change
Expand Up @@ -1256,6 +1256,9 @@ def start_tls(
ssl_stream._ssl_connect_future = future
ssl_stream.max_buffer_size = self.max_buffer_size
ssl_stream.read_chunk_size = self.read_chunk_size
# Keep a reference so callers can clean up if the handshake
# is abandoned (e.g. due to timeout). See tornado#3614.
self._tls_stream = ssl_stream
return future

def _handle_connect(self) -> None:
Expand Down
22 changes: 16 additions & 6 deletions tornado/tcpclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,22 @@ async def connect(
# the same host. (http://tools.ietf.org/html/rfc6555#section-4.2)
if ssl_options is not None:
if timeout is not None:
stream = await gen.with_timeout(
timeout,
stream.start_tls(
False, ssl_options=ssl_options, server_hostname=host
),
)
try:
stream = await gen.with_timeout(
timeout,
stream.start_tls(
False, ssl_options=ssl_options, server_hostname=host
),
)
except gen.TimeoutError:
# start_tls() transferred socket ownership to a new
# SSLIOStream (self._tls_stream). Close it to prevent
# a file descriptor leak. See tornado#3614.
tls_stream = getattr(stream, "_tls_stream", None)
if tls_stream is not None:
tls_stream.close()
stream._tls_stream = None
raise
else:
stream = await stream.start_tls(
False, ssl_options=ssl_options, server_hostname=host
Expand Down
61 changes: 61 additions & 0 deletions tornado/test/tcpclient_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import getpass
import os
import socket
import typing
import unittest
Expand Down Expand Up @@ -428,3 +429,63 @@ def test_two_family_timeout_after_connect_timeout(self):
self.assertEqual(len(conn.streams), 1)
self.assert_connector_streams_closed(conn)
self.assertRaises(TimeoutError, future.result)


class TestTLSHandshakeTimeoutCleanup(AsyncTestCase):
"""Regression test: TLS handshake timeout must not leak file descriptors.

When TCPClient.connect() times out during the TLS handshake phase,
start_tls() has already transferred socket ownership to a new SSLIOStream.
The timeout handler must close that SSLIOStream to prevent an fd leak.
See tornado#3614.
"""

@gen_test
def test_tls_handshake_timeout_closes_stream(self) -> None:
import ssl as _ssl

# Set up a server socket that accepts TCP but never completes TLS
server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
server_sock.bind(("127.0.0.1", 0))
port = server_sock.getsockname()[1]
server_sock.listen(1)

ctx = _ssl.SSLContext(_ssl.PROTOCOL_TLS_CLIENT)
ctx.check_hostname = False
ctx.verify_mode = _ssl.CERT_NONE

# Count open fds before (Linux /proc/self/fd)
try:
fds_before = len(os.listdir("/proc/self/fd"))
except OSError:
fds_before = 0

client = TCPClient()
error_raised = False
try:
yield client.connect(
"127.0.0.1",
port,
ssl_options=ctx,
timeout=0.01,
)
except TimeoutError:
error_raised = True

self.assertTrue(error_raised, "Expected TimeoutError from TLS handshake timeout")

server_sock.close()

# Let IOLoop process cleanup callbacks
from tornado.gen import moment
yield moment

# Verify no fd leaked
if fds_before > 0:
fds_after = len(os.listdir("/proc/self/fd"))
self.assertLessEqual(
fds_after, fds_before + 2,
"File descriptor leaked after TLS handshake timeout "
f"(before={fds_before}, after={fds_after})",
)