THRIFT-3634 Fix Python TSocket resource leak on connection failure
Client: Python
Patch: Nobuaki Sukegawa
This closes #867
diff --git a/lib/py/src/transport/TSSLSocket.py b/lib/py/src/transport/TSSLSocket.py
index a66461a..4cae27c 100644
--- a/lib/py/src/transport/TSSLSocket.py
+++ b/lib/py/src/transport/TSSLSocket.py
@@ -275,37 +275,18 @@
DeprecationWarning, stacklevel=2)
self.cert_reqs = ssl.CERT_REQUIRED if value else ssl.CERT_NONE
- def open(self):
+ def _do_open(self, family, socktype):
+ plain_sock = socket.socket(family, socktype)
try:
- addrs = self._resolveAddr()
- for addr in addrs:
- sock_family, sock_type, _, _, ip_port = addr
- plain_sock = socket.socket(sock_family, sock_type)
- self.handle = self._wrap_socket(plain_sock)
- self.handle.settimeout(self._timeout)
- try:
- self.handle.connect(ip_port)
- except socket.error:
- self.handle.close()
- if addr is not addrs[-1]:
- logger.warning(
- 'Error while connecting with %s. Trying next one.',
- ip_port, exc_info=True)
- continue
- else:
- raise
- break
- except socket.error as e:
- if self._unix_socket:
- message = 'Could not connect to secure socket %s: %s' \
- % (self._unix_socket, e)
- else:
- message = 'Could not connect to %s:%d: %s' \
- % (self.host, self.port, e)
- logger.exception(
- 'Error while connecting with %s.', ip_port)
- raise TTransportException(TTransportException.NOT_OPEN, message)
+ return self._wrap_socket(plain_sock)
+ except Exception:
+ plain_sock.close()
+ msg = 'failed to initialize SSL'
+ logger.exception(msg)
+ raise TTransportException(TTransportException.NOT_OPEN, msg)
+ def open(self):
+ super(TSSLSocket, self).open()
if self._should_verify:
self.peercert = self.handle.getpeercert()
try:
diff --git a/lib/py/src/transport/TSocket.py b/lib/py/src/transport/TSocket.py
index a8ed4b7..c91de9d 100644
--- a/lib/py/src/transport/TSocket.py
+++ b/lib/py/src/transport/TSocket.py
@@ -18,12 +18,15 @@
#
import errno
+import logging
import os
import socket
import sys
from .TTransport import TTransportBase, TTransportException, TServerTransportBase
+logger = logging.getLogger(__name__)
+
class TSocketBase(TTransportBase):
def _resolveAddr(self):
@@ -78,27 +81,36 @@
if self.handle is not None:
self.handle.settimeout(self._timeout)
+ def _do_open(self, family, socktype):
+ return socket.socket(family, socktype)
+
+ @property
+ def _address(self):
+ return self._unix_socket if self._unix_socket else '%s:%d' % (self.host, self.port)
+
def open(self):
+ if self.handle:
+ raise TTransportException(TTransportException.ALREADY_OPEN)
try:
- res0 = self._resolveAddr()
- for res in res0:
- self.handle = socket.socket(res[0], res[1])
- self.handle.settimeout(self._timeout)
- try:
- self.handle.connect(res[4])
- except socket.error as e:
- if res is not res0[-1]:
- continue
- else:
- raise e
- break
- except socket.error as e:
- if self._unix_socket:
- message = 'Could not connect to socket %s' % self._unix_socket
- else:
- message = 'Could not connect to %s:%d' % (self.host, self.port)
- raise TTransportException(type=TTransportException.NOT_OPEN,
- message=message)
+ addrs = self._resolveAddr()
+ except socket.gaierror:
+ msg = 'failed to resolve sockaddr for ' + str(self._address)
+ logger.exception(msg)
+ raise TTransportException(TTransportException.NOT_OPEN, msg)
+ for family, socktype, _, _, sockaddr in addrs:
+ handle = self._do_open(family, socktype)
+ handle.settimeout(self._timeout)
+ try:
+ handle.connect(sockaddr)
+ self.handle = handle
+ return
+ except socket.error:
+ handle.close()
+ logger.info('Could not connect to %s', sockaddr, exc_info=True)
+ msg = 'Could not connect to any of %s' % list(map(lambda a: a[4],
+ addrs))
+ logger.error(msg)
+ raise TTransportException(TTransportException.NOT_OPEN, msg)
def read(self, sz):
try: