Commit 4fb1973c authored by Jason Madden's avatar Jason Madden

Stop artificially catching ConnectionResetError in ssl3.SSLSocket.read

And tweak the way we find open connections on linux; psutil considers some things closed even when the fd is still valid.

Fixes #1637
parent c357996d
Python 3 ``gevent.ssl.SSLSocket`` objects no longer attempt to catch
``ConnectionResetError`` and treat it the same as an ``SSLError`` with
``SSL_ERROR_EOF`` (typically by suppressing it).
This was a difference from the way the standard library behaved (which
is to raise the exception). It was added to gevent during early
testing of OpenSSL 1.1 and TLS 1.3.
......@@ -44,6 +44,8 @@ if [ -d /gevent -a -d /opt/python ]; then
export XDG_CACHE_HOME="/cache"
ls -ld /cache
yum -y install libffi-devel ccache
# On Fedora Rawhide (F33)
# yum install python39 python3-devel gcc kernel-devel kernel-headers make diffutils file
mkdir /tmp/build
cd /tmp/build
......
......@@ -396,15 +396,15 @@ class SSLSocket(socket):
if ex.args[0] == SSL_ERROR_EOF and self.suppress_ragged_eofs:
return b'' if buffer is None else len(buffer) - initial_buf_len
raise
except ConnectionResetError:
# Certain versions of Python, built against certain
# versions of OpenSSL operating in certain modes,
# can produce this instead of SSLError. Notably, it looks
# like anything built against 1.1.1c do?
if self.suppress_ragged_eofs:
return b'' if buffer is None else len(buffer) - initial_buf_len
raise
# Certain versions of Python, built against certain
# versions of OpenSSL operating in certain modes, can
# produce ``ConnectionResetError`` instead of
# ``SSLError``. Notably, it looks like anything built
# against 1.1.1c does that? gevent briefly (from support of TLS 1.3
# in Sept 2019 to issue #1637 it June 2020) caught that error and treaded
# it just like SSL_ERROR_EOF. But that's not what the standard library does.
# So presumably errors that result from unexpected ``ConnectionResetError``
# are issues in gevent tests.
def write(self, data):
"""Write DATA to the underlying SSL channel. Returns
......
......@@ -49,14 +49,14 @@ def _collects(func):
# collected and drop references and thus close files. We should be deterministic
# and careful about closing things.
@functools.wraps(func)
def f():
def f(**kw):
gc.collect()
gc.collect()
enabled = gc.isenabled()
gc.disable()
try:
return func()
return func(**kw)
finally:
if enabled:
gc.enable()
......@@ -82,7 +82,7 @@ else:
os.remove(tmpname)
return data
def default_get_open_files(pipes=False):
def default_get_open_files(pipes=False, **_kwargs):
data = _run_lsof()
results = {}
for line in data.split('\n'):
......@@ -128,9 +128,13 @@ except ImportError:
get_open_files = default_get_open_files
get_number_open_files = default_get_number_open_files
else:
class _TrivialOpenFile(object):
__slots__ = ('fd',)
def __init__(self, fd):
self.fd = fd
@_collects
def get_open_files():
def get_open_files(count_closing_as_open=True, **_kw):
"""
Return a list of popenfile and pconn objects.
......@@ -146,8 +150,27 @@ else:
for _ in range(3):
try:
if count_closing_as_open and os.path.exists('/proc/'):
# Linux only.
# psutil doesn't always see all connections, even though
# they exist in the filesystem. It's not entirely clear why.
# It sees them on Travis (prior to Ubuntu Bionic, at least)
# but doesn't in the manylinux image or Fedora 33 Rawhide image.
# This happens in test__makefile_ref TestSSL.*; in particular, if a
# ``sslsock.makefile()`` is opened and used to read all data, and the sending
# side shuts down, psutil no longer finds the open file. So we add them
# back in.
#
# Of course, the flip side of this is that we sometimes find connections
# we're not expecting.
# I *think* this has to do with CLOSE_WAIT handling?
fd_directory = '/proc/%d/fd' % os.getpid()
fd_files = os.listdir(fd_directory)
else:
fd_files = []
process = psutil.Process()
results['data'] = process.open_files() + process.connections('all')
results['data'] = process.open_files()
results['data'] += process.connections('all')
break
except OSError:
pass
......@@ -157,7 +180,12 @@ else:
for x in results['data']:
results[x.fd] = x
results['data'] += ['From psutil', process]
for fd_str in fd_files:
if fd_str not in results:
fd = int(fd_str)
results[fd] = _TrivialOpenFile(fd)
results['data'] += [('From psutil', process)]
results['data'] += [('fd files', fd_files)]
return results
@_collects
......
......@@ -32,6 +32,13 @@ class Test(unittest.TestCase):
)
BACKENDS_THAT_WILL_FAIL_TO_CREATE_AT_RUNTIME = (
# This fails on the Ubuntu Bionic image, and on
# the Fedora Rawhide 33 image. It's not clear why; needs
# investigated.
'linux_iouring',
)
BACKENDS_THAT_WILL_FAIL_TO_CREATE_AT_RUNTIME += (
# This can be compiled on any (?) version of
# linux, but there's a runtime check that you're
# running at least kernel 4.19, so we can fail to create
......
......@@ -12,7 +12,6 @@ import gevent.testing as greentest
from gevent.testing.params import DEFAULT_BIND_ADDR_TUPLE
from gevent.testing.params import DEFAULT_CONNECT
from gevent.testing.sockets import tcp_listener
from gevent.testing.skipping import skipOnManylinux
dirname = os.path.dirname(os.path.abspath(__file__))
certfile = os.path.join(dirname, '2_7_keycert.pem')
......@@ -75,7 +74,9 @@ class Test(greentest.TestCase):
def assert_fd_closed(self, fileno):
assert isinstance(fileno, fd_types), repr(fileno)
assert fileno > 0, fileno
open_files = get_open_files()
# Here, if we're in the process of closing, don't consider it open.
# This goes into details of psutil
open_files = get_open_files(count_closing_as_open=False)
if fileno in open_files:
raise AssertionError('%r is not closed:\n%s' % (fileno, open_files['data']))
......@@ -248,7 +249,6 @@ class TestSocket(Test):
@greentest.skipOnAppVeyor("This sometimes times out for no apparent reason.")
@skipOnManylinux("For some reason manylinux doesn't see the open files all the time.")
class TestSSL(Test):
def _ssl_connect_task(self, connector, port, accepted_event):
......@@ -409,7 +409,6 @@ class TestSSL(Test):
f.close()
self.assert_closed(client_socket, fileno)
@skipOnManylinux("Doesn't see the file open")
def test_serverssl_makefile2(self):
raw_listener = tcp_listener(backlog=1)
port = raw_listener.getsockname()[1]
......@@ -439,16 +438,17 @@ class TestSSL(Test):
self.assert_open(client_socket, fileno)
self.assertEqual(f.read(), 'test_serverssl_makefile2')
self.assertEqual(f.read(), '')
# Closing file object does not close the socket.
f.close()
if WIN and psutil:
# Hmm?
self.extra_allowed_open_states = (psutil.CONN_CLOSE_WAIT,)
self.assert_open(client_socket, fileno)
client_socket.close()
self.assert_closed(client_socket, fileno)
class Closing(object):
def __init__(self, *init):
......
......@@ -3,7 +3,8 @@
from __future__ import print_function
from __future__ import absolute_import
from gevent import monkey; monkey.patch_all()
from gevent import monkey
monkey.patch_all()
import sys
import array
......@@ -149,11 +150,11 @@ class TestTCP(greentest.TestCase):
conn, _ = self.listener.accept()
try:
with conn.makefile(mode='rb') as r:
log("\taccepted on server", conn)
log("\taccepted on server; client conn is", conn, "file is", r)
accepted_event.set()
log("\treading")
read_data.append(r.read())
log("\tdone reading")
log("\tdone reading", r, "got bytes", len(read_data[0]))
del r
finally:
conn.close()
......@@ -173,8 +174,11 @@ class TestTCP(greentest.TestCase):
# sockets interferes, especially when using SSL sockets.
# The best way to get a decent FIN to the server is to shutdown
# the output. Doing that on Python 3, OTOH, is contraindicated
# except on PyPy.
should_shutdown = greentest.PY2 or greentest.PYPY
# except on PyPy, so this used to read ``PY2 or PYPY``. But
# it seems that a shutdown is generally good practice, and I didn't
# document what errors we saw without it. Per issue #1637
# lets do a shutdown everywhere.
should_shutdown = 1
# It's important to wait for the server to fully accept before
# we shutdown and close the socket. In SSL mode, the number
......@@ -192,9 +196,10 @@ class TestTCP(greentest.TestCase):
# when it got switched to by server.join(), found its new socket
# dead.
accepted_event.wait()
log("accepted", client)
log("Client got accepted event from server", client, "; sending data", len(data))
try:
getattr(client, client_method)(data)
x = getattr(client, client_method)(data)
log("Client sent data: result from method", x)
except:
# unwrapping might not work after this because we're in
# a bad state.
......@@ -204,7 +209,7 @@ class TestTCP(greentest.TestCase):
should_shutdown = False
raise
finally:
log("shutdown")
log("Client will: Shutdown?", should_shutdown, "Unwrap?", should_unwrap)
if should_shutdown:
client.shutdown(socket.SHUT_RDWR)
elif should_unwrap:
......@@ -218,8 +223,9 @@ class TestTCP(greentest.TestCase):
pass
else:
raise
log("closing")
log("Client will close")
client.close()
gevent.idle()
finally:
server.join(10)
assert not server.is_alive()
......@@ -231,6 +237,8 @@ class TestTCP(greentest.TestCase):
match_data = self.long_data
read_data = read_data[0].split(b',')
match_data = match_data.split(b',')
self.assertEqual(read_data[0], match_data[0])
self.assertEqual(len(read_data), len(match_data))
self.assertEqual(read_data, match_data)
def test_sendall_str(self):
......
from __future__ import print_function, division, absolute_import
from gevent import monkey; monkey.patch_all()
from gevent import monkey
monkey.patch_all()
import os
import socket
......
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