Commit 61399d52 authored by Jason Madden's avatar Jason Madden

Fix #1044 by always closing opened sockets before raising

Also enable ResoureWarnings by default in the test suite and fix a
bunch that showed up.
parent be540604
......@@ -90,6 +90,12 @@
- ``socket.send()`` now catches ``EPROTYPE`` on macOS to handle a race
condition during shutdown. Fixed in :pr:`1035` by Jay Oster.
- :func:`gevent.socket.create_connection` now properly cleans up open
sockets if connecting or binding raises a :exc:`BaseException` like
:exc:`KeyboardInterrupt`, :exc:`greenlet.GreenletExit` or
:exc:`gevent.timeout.Timeout`. Reported in :issue:`1044` by
kochelmonster.
- Update c-ares to 1.13.0. See :issue:`990`.
1.2.2 (2017-06-05)
......
......@@ -716,8 +716,13 @@ def main():
def _get_script_help():
from inspect import getargspec
patch_all_args = getargspec(patch_all)[0] # pylint:disable=deprecated-method
# pylint:disable=deprecated-method
import inspect
try:
getter = inspect.getfullargspec # deprecated in 3.5, un-deprecated in 3.6
except AttributeError:
getter = inspect.getargspec
patch_all_args = getter(patch_all)[0]
modules = [x for x in patch_all_args if 'patch_' + x in globals()]
script_help = """gevent.monkey - monkey patch the standard modules to use gevent.
......
......@@ -74,7 +74,13 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT, source_address=N
host, port = address
err = None
for res in getaddrinfo(host, port, 0 if has_ipv6 else AF_INET, SOCK_STREAM):
# getaddrinfo is documented as returning a list, but our interface
# is pluggable, so be sure it does.
addrs = list(getaddrinfo(host, port, 0 if has_ipv6 else AF_INET, SOCK_STREAM))
if not addrs:
raise error("getaddrinfo returns an empty list")
for res in addrs:
af, socktype, proto, _, sa = res
sock = None
try:
......@@ -84,24 +90,34 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT, source_address=N
if source_address:
sock.bind(source_address)
sock.connect(sa)
return sock
except error as ex:
# without exc_clear(), if connect() fails once, the socket is referenced by the frame in exc_info
# and the next bind() fails (see test__socket.TestCreateConnection)
# that does not happen with regular sockets though, because _socket.socket.connect() is a built-in.
# this is similar to "getnameinfo loses a reference" failure in test_socket.py
if not PY3:
sys.exc_clear() # pylint:disable=no-member,useless-suppression
except error:
if sock is not None:
sock.close()
err = ex
if err is not None:
try:
raise err # pylint:disable=raising-bad-type
finally:
err = None
else:
raise error("getaddrinfo returns an empty list")
if res is addrs[-1]:
raise
# without exc_clear(), if connect() fails once, the socket
# is referenced by the frame in exc_info and the next
# bind() fails (see test__socket.TestCreateConnection)
# that does not happen with regular sockets though,
# because _socket.socket.connect() is a built-in. this is
# similar to "getnameinfo loses a reference" failure in
# test_socket.py
try:
c = sys.exc_clear
except AttributeError:
pass # Python 3 doesn't have this
else:
c()
except BaseException:
# Things like GreenletExit, Timeout and KeyboardInterrupt.
# These get raised immediately, being sure to
# close the socket
if sock is not None:
sock.close()
raise
else:
return sock
# This is promised to be in the __all__ of the _source, but, for circularity reasons,
# we implement it in this module. Mostly for documentation purposes, put it
......
......@@ -430,6 +430,10 @@ class TestCase(TestCaseMetaClass("NewBase", (BaseTestCase,), {})):
super(TestCase, cls).tearDownClass()
def _close_on_teardown(self, resource):
"""
*resource* either has a ``close`` method, or is a
callable.
"""
if 'close_on_teardown' not in self.__dict__:
self.close_on_teardown = []
self.close_on_teardown.append(resource)
......
......@@ -47,17 +47,18 @@ def TESTRUNNER(tests=None):
if tests:
atexit.register(os.system, 'rm -f */@test*')
basic_args = [sys.executable, '-u', '-W', 'ignore', '-m' 'monkey_test']
for filename in tests:
if filename in version_tests:
util.log("Overriding %s from %s with file from %s", filename, directory, full_directory)
continue
yield [sys.executable, '-u', '-m', 'monkey_test', filename], options.copy()
yield [sys.executable, '-u', '-m', 'monkey_test', '--Event', filename], options.copy()
yield basic_args + [filename], options.copy()
yield basic_args + ['--Event', filename], options.copy()
options['cwd'] = full_directory
for filename in version_tests:
yield [sys.executable, '-u', '-m', 'monkey_test', filename], options.copy()
yield [sys.executable, '-u', '-m', 'monkey_test', '--Event', filename], options.copy()
yield basic_args + [filename], options.copy()
yield basic_args + ['--Event', filename], options.copy()
def main():
......
......@@ -17,7 +17,8 @@ class Test_udp_client(TestCase):
server = DatagramServer('127.0.0.1:9000', handle)
server.start()
try:
run([sys.executable, '-u', 'udp_client.py', 'Test_udp_client'], timeout=10, cwd='../../examples/')
run([sys.executable, '-W', 'ignore' '-u', 'udp_client.py', 'Test_udp_client'],
timeout=10, cwd='../../examples/')
finally:
server.close()
self.assertEqual(log, [b'Test_udp_client'])
......
......@@ -18,6 +18,7 @@
# THE SOFTWARE.
from greentest import TestCase, main, tcp_listener
from greentest import skipOnPyPy
import gevent
from gevent import socket
import sys
......@@ -87,17 +88,19 @@ class TestGreenIo(TestCase):
did_it_work(server)
server_greenlet.kill()
@skipOnPyPy("GC is different")
def test_del_closes_socket(self):
if PYPY:
return
timer = gevent.Timeout.start_new(0.5)
def accept_once(listener):
# delete/overwrite the original conn
# object, only keeping the file object around
# closing the file object should close everything
# XXX: This is not exactly true on Python 3.
# This produces a ResourceWarning.
try:
conn, addr = listener.accept()
conn, _ = listener.accept()
conn = conn.makefile(mode='wb')
conn.write(b'hello\n')
conn.close()
......
......@@ -32,14 +32,13 @@ DELAY = 0.1
class TestCloseSocketWhilePolling(greentest.TestCase):
def test(self):
try:
with self.assertRaises(Exception):
sock = socket.socket()
self._close_on_teardown(sock)
get_hub().loop.timer(0, sock.close)
sock.connect(('python.org', 81))
except Exception:
gevent.sleep(0)
else:
assert False, 'expected an error here'
gevent.sleep(0)
class TestExceptionInMainloop(greentest.TestCase):
......
from __future__ import print_function
import os
from gevent import monkey; monkey.patch_all()
import re
import socket
import ssl
import threading
import unittest
import errno
from greentest import TestCase
dirname = os.path.dirname(os.path.abspath(__file__))
certfile = os.path.join(dirname, '2.7/keycert.pem')
pid = os.getpid()
......@@ -27,13 +28,13 @@ except ImportError:
psutil = None
class Test(unittest.TestCase):
class Test(TestCase):
extra_allowed_open_states = ()
def tearDown(self):
self.extra_allowed_open_states = ()
unittest.TestCase.tearDown(self)
super(Test, self).tearDown()
def assert_raises_EBADF(self, func):
try:
......@@ -156,6 +157,7 @@ class TestSocket(Test):
listener.listen(1)
connector = socket.socket()
self._close_on_teardown(connector)
def connect():
connector.connect(('127.0.0.1', port))
......@@ -180,6 +182,7 @@ class TestSocket(Test):
listener.listen(1)
connector = socket.socket()
self._close_on_teardown(connector)
def connect():
connector.connect(('127.0.0.1', port))
......@@ -213,6 +216,7 @@ class TestSocket(Test):
listener.listen(1)
connector = socket.socket()
self._close_on_teardown(connector)
def connect():
connector.connect(('127.0.0.1', port))
......@@ -282,10 +286,12 @@ class TestSSL(Test):
listener.listen(1)
connector = socket.socket()
self._close_on_teardown(connector)
def connect():
connector.connect(('127.0.0.1', port))
ssl.wrap_socket(connector)
x = ssl.wrap_socket(connector)
self._close_on_teardown(x)
t = threading.Thread(target=connect)
t.start()
......@@ -303,15 +309,18 @@ class TestSSL(Test):
def test_server_makefile1(self):
listener = socket.socket()
self._close_on_teardown(listener)
listener.bind(('127.0.0.1', 0))
port = listener.getsockname()[1]
listener.listen(1)
connector = socket.socket()
self._close_on_teardown(connector)
def connect():
connector.connect(('127.0.0.1', port))
ssl.wrap_socket(connector)
x = ssl.wrap_socket(connector)
self._close_on_teardown(x)
t = threading.Thread(target=connect)
t.start()
......@@ -338,10 +347,12 @@ class TestSSL(Test):
listener.listen(1)
connector = socket.socket()
self._close_on_teardown(connector)
def connect():
connector.connect(('127.0.0.1', port))
ssl.wrap_socket(connector)
x = ssl.wrap_socket(connector)
self._close_on_teardown(x)
t = threading.Thread(target=connect)
t.start()
......@@ -372,10 +383,12 @@ class TestSSL(Test):
listener = ssl.wrap_socket(listener, keyfile=certfile, certfile=certfile)
connector = socket.socket()
self._close_on_teardown(connector)
def connect():
connector.connect(('127.0.0.1', port))
ssl.wrap_socket(connector)
x = ssl.wrap_socket(connector)
self._close_on_teardown(x)
t = threading.Thread(target=connect)
t.start()
......
......@@ -70,7 +70,7 @@ def init_server():
def handle_request(s, raise_on_timeout):
try:
conn, address = s.accept()
conn, _ = s.accept()
except socket.timeout:
if raise_on_timeout:
raise
......@@ -83,7 +83,7 @@ def handle_request(s, raise_on_timeout):
res = conn.send(b'bye')
#print('handle_request - sent %r' % res)
#print('handle_request - conn refcount: %s' % sys.getrefcount(conn))
#conn.close()
conn.close()
def make_request(port):
......@@ -96,7 +96,7 @@ def make_request(port):
res = s.recv(100)
assert res == b'bye', repr(res)
#print('make_request - recvd %r' % res)
#s.close()
s.close()
def run_interaction(run_client):
......
......@@ -9,8 +9,11 @@ try:
assert weakref.ref(Dummy())() is None
from gevent import socket
assert weakref.ref(socket.socket())() is None
s = socket.socket()
r = weakref.ref(s)
s.close()
del s
assert r() is None
except AssertionError:
import sys
if hasattr(sys, 'pypy_version_info'):
......
......@@ -322,19 +322,59 @@ def get_port():
class TestCreateConnection(greentest.TestCase):
__timeout__ = 5
__timeout__ = 5000
def test(self):
try:
def test_refuses(self):
with self.assertRaises(socket.error) as cm:
socket.create_connection((greentest.DEFAULT_BIND_ADDR, get_port()),
timeout=30,
source_address=('', get_port()))
except socket.error as ex:
if 'refused' not in str(ex).lower():
raise
else:
raise AssertionError('create_connection did not raise socket.error as expected')
ex = cm.exception
self.assertIn('refused', str(ex).lower())
def test_base_exception(self):
# such as a GreenletExit or a gevent.timeout.Timeout
class E(BaseException):
pass
class MockSocket(object):
created = ()
closed = False
def __init__(self, *_):
MockSocket.created += (self,)
def connect(self, _):
raise E()
def close(self):
self.closed = True
def mockgetaddrinfo(*_):
return [(1, 2, 3, 3, 5),]
import gevent.socket as gsocket
# Make sure we're monkey patched
self.assertEqual(gsocket.create_connection, socket.create_connection)
orig_socket = gsocket.socket
orig_getaddrinfo = gsocket.getaddrinfo
try:
gsocket.socket = MockSocket
gsocket.getaddrinfo = mockgetaddrinfo
with self.assertRaises(E):
socket.create_connection(('host', 'port'))
self.assertEqual(1, len(MockSocket.created))
self.assertTrue(MockSocket.created[0].closed)
finally:
MockSocket.created = ()
gsocket.socket = orig_socket
gsocket.getaddrinfo = orig_getaddrinfo
class TestFunctions(greentest.TestCase):
......
......@@ -34,6 +34,7 @@ class TestSocketErrors(greentest.TestCase):
def test_connection_refused(self):
s = socket()
self._close_on_teardown(s)
try:
s.connect(('127.0.0.1', 81))
except error as ex:
......
......@@ -12,12 +12,13 @@ def _send(socket):
sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
sock.connect(('127.0.0.1', 12345))
getattr(sock, meth)(anStructure)
sock.close()
sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
sock.connect(('127.0.0.1', 12345))
sock.settimeout(1.0)
getattr(sock, meth)(anStructure)
sock.close()
def TestSendBuiltinSocket():
import socket
......
......@@ -5,36 +5,42 @@ import greentest
class Test(greentest.TestCase):
def start(self):
server = None
acceptor = None
server_port = None
def _accept(self):
conn, _ = self.server.accept()
self._close_on_teardown(conn)
def setUp(self):
super(Test, self).setUp()
self.server = socket.socket()
self._close_on_teardown(self.server)
self.server.bind(('127.0.0.1', 0))
self.server.listen(1)
self.server_port = self.server.getsockname()[1]
self.acceptor = gevent.spawn(self.server.accept)
self.acceptor = gevent.spawn(self._accept)
def stop(self):
self.server.close()
def tearDown(self):
self.acceptor.kill()
self.server.close()
del self.acceptor
del self.server
super(Test, self).tearDown()
def test(self):
self.start()
try:
sock = socket.socket()
sock.connect(('127.0.0.1', self.server_port))
try:
sock.settimeout(0.1)
try:
result = sock.recv(1024)
raise AssertionError('Expected timeout to be raised, instead recv() returned %r' % (result, ))
except socket.error as ex:
self.assertEqual(ex.args, ('timed out',))
self.assertEqual(str(ex), 'timed out')
finally:
sock.close()
finally:
self.stop()
sock = socket.socket()
self._close_on_teardown(sock)
sock.connect(('127.0.0.1', self.server_port))
sock.settimeout(0.1)
with self.assertRaises(socket.error) as cm:
sock.recv(1024)
ex = cm.exception
self.assertEqual(ex.args, ('timed out',))
self.assertEqual(str(ex), 'timed out')
if __name__ == '__main__':
......
......@@ -71,7 +71,8 @@ class Test(greentest.TestCase):
def test_communicate(self):
p = subprocess.Popen([sys.executable, "-c",
p = subprocess.Popen([sys.executable, "-W", "ignore",
"-c",
'import sys,os;'
'sys.stderr.write("pineapple");'
'sys.stdout.write(sys.stdin.read())'],
......@@ -91,7 +92,9 @@ class Test(greentest.TestCase):
# Native string all the things. See https://github.com/gevent/gevent/issues/1039
p = subprocess.Popen(
[
sys.executable, "-c",
sys.executable,
"-W", "ignore",
"-c",
'import sys,os;'
'sys.stderr.write("pineapple\\r\\n\\xff\\xff\\xf2\\xf9\\r\\n");'
'sys.stdout.write(sys.stdin.read())'
......
This diff is collapsed.
......@@ -297,6 +297,16 @@ def main():
config_data = f.read()
six.exec_(config_data, config)
FAILING_TESTS = config['FAILING_TESTS']
if 'PYTHONWARNINGS' not in os.environ and not sys.warnoptions:
# Enable default warnings such as ResourceWarning.
# On Python 3[.6], the system site.py module has
# "open(fullname, 'rU')" which produces the warning that
# 'U' is deprecated, so ignore warnings from site.py
os.environ['PYTHONWARNINGS'] = 'default,ignore:::site:'
if 'PYTHONFAULTHANDLER' not in os.environ:
os.environ['PYTHONFAULTHANDLER'] = 'true'
tests = discover(options.tests, options.ignore, coverage)
if options.discover:
for cmd, options in tests:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment