mirror of
https://github.com/python/cpython.git
synced 2026-01-06 15:32:22 +00:00
[3.13] gh-137583: Only lock the SSL context, not the SSL socket (GH-137588) (GH-137613)
Fixes a deadlock introduced in 3.13.6.
(cherry picked from commit 55788a9096)
This commit is contained in:
parent
c1e1c880ee
commit
da39cb9716
4 changed files with 57 additions and 17 deletions
|
|
@ -4584,6 +4584,42 @@ def server_callback(identity):
|
|||
with client_context.wrap_socket(socket.socket()) as s:
|
||||
s.connect((HOST, server.port))
|
||||
|
||||
def test_thread_recv_while_main_thread_sends(self):
|
||||
# GH-137583: Locking was added to calls to send() and recv() on SSL
|
||||
# socket objects. This seemed fine at the surface level because those
|
||||
# calls weren't re-entrant, but recv() calls would implicitly mimick
|
||||
# holding a lock by blocking until it received data. This means that
|
||||
# if a thread started to infinitely block until data was received, calls
|
||||
# to send() would deadlock, because it would wait forever on the lock
|
||||
# that the recv() call held.
|
||||
data = b"1" * 1024
|
||||
event = threading.Event()
|
||||
def background(sock):
|
||||
event.set()
|
||||
received = sock.recv(len(data))
|
||||
self.assertEqual(received, data)
|
||||
|
||||
client_context, server_context, hostname = testing_context()
|
||||
server = ThreadedEchoServer(context=server_context)
|
||||
with server:
|
||||
with client_context.wrap_socket(socket.socket(),
|
||||
server_hostname=hostname) as sock:
|
||||
sock.connect((HOST, server.port))
|
||||
sock.settimeout(1)
|
||||
sock.setblocking(1)
|
||||
# Ensure that the server is ready to accept requests
|
||||
sock.sendall(b"123")
|
||||
self.assertEqual(sock.recv(3), b"123")
|
||||
with threading_helper.catch_threading_exception() as cm:
|
||||
thread = threading.Thread(target=background,
|
||||
args=(sock,), daemon=True)
|
||||
thread.start()
|
||||
event.wait()
|
||||
sock.sendall(data)
|
||||
thread.join()
|
||||
if cm.exc_value is not None:
|
||||
raise cm.exc_value
|
||||
|
||||
|
||||
@unittest.skipUnless(has_tls_version('TLSv1_3'), "Test needs TLS 1.3")
|
||||
class TestPostHandshakeAuth(unittest.TestCase):
|
||||
|
|
|
|||
|
|
@ -0,0 +1,4 @@
|
|||
Fix a deadlock introduced in 3.13.6 when a call to
|
||||
:meth:`ssl.SSLSocket.recv <socket.socket.recv>` was blocked in one thread,
|
||||
and then another method on the object (such as :meth:`ssl.SSLSocket.send <socket.socket.send>`)
|
||||
was subsequently called in another thread.
|
||||
|
|
@ -332,9 +332,6 @@ typedef struct {
|
|||
* and shutdown methods check for chained exceptions.
|
||||
*/
|
||||
PyObject *exc;
|
||||
/* Lock to synchronize calls when the thread state is detached.
|
||||
See also gh-134698. */
|
||||
PyMutex tstate_mutex;
|
||||
} PySSLSocket;
|
||||
|
||||
typedef struct {
|
||||
|
|
@ -846,7 +843,6 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
|
|||
self->server_hostname = NULL;
|
||||
self->err = err;
|
||||
self->exc = NULL;
|
||||
self->tstate_mutex = (PyMutex){0};
|
||||
|
||||
/* Make sure the SSL error state is initialized */
|
||||
ERR_clear_error();
|
||||
|
|
@ -919,12 +915,12 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
|
|||
BIO_set_nbio(SSL_get_wbio(self->ssl), 1);
|
||||
}
|
||||
|
||||
PySSL_BEGIN_ALLOW_THREADS(self)
|
||||
Py_BEGIN_ALLOW_THREADS;
|
||||
if (socket_type == PY_SSL_CLIENT)
|
||||
SSL_set_connect_state(self->ssl);
|
||||
else
|
||||
SSL_set_accept_state(self->ssl);
|
||||
PySSL_END_ALLOW_THREADS(self)
|
||||
Py_END_ALLOW_THREADS;
|
||||
|
||||
self->socket_type = socket_type;
|
||||
if (sock != NULL) {
|
||||
|
|
@ -993,10 +989,11 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
|
|||
/* Actually negotiate SSL connection */
|
||||
/* XXX If SSL_do_handshake() returns 0, it's also a failure. */
|
||||
do {
|
||||
PySSL_BEGIN_ALLOW_THREADS(self)
|
||||
Py_BEGIN_ALLOW_THREADS
|
||||
ret = SSL_do_handshake(self->ssl);
|
||||
err = _PySSL_errno(ret < 1, self->ssl, ret);
|
||||
PySSL_END_ALLOW_THREADS(self)
|
||||
Py_END_ALLOW_THREADS;
|
||||
_PySSL_FIX_ERRNO;
|
||||
self->err = err;
|
||||
|
||||
if (PyErr_CheckSignals())
|
||||
|
|
@ -2462,10 +2459,11 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
|
|||
}
|
||||
|
||||
do {
|
||||
PySSL_BEGIN_ALLOW_THREADS(self)
|
||||
Py_BEGIN_ALLOW_THREADS;
|
||||
retval = SSL_write_ex(self->ssl, b->buf, (size_t)b->len, &count);
|
||||
err = _PySSL_errno(retval == 0, self->ssl, retval);
|
||||
PySSL_END_ALLOW_THREADS(self)
|
||||
Py_END_ALLOW_THREADS;
|
||||
_PySSL_FIX_ERRNO;
|
||||
self->err = err;
|
||||
|
||||
if (PyErr_CheckSignals())
|
||||
|
|
@ -2523,10 +2521,11 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
|
|||
int count = 0;
|
||||
_PySSLError err;
|
||||
|
||||
PySSL_BEGIN_ALLOW_THREADS(self)
|
||||
Py_BEGIN_ALLOW_THREADS;
|
||||
count = SSL_pending(self->ssl);
|
||||
err = _PySSL_errno(count < 0, self->ssl, count);
|
||||
PySSL_END_ALLOW_THREADS(self)
|
||||
Py_END_ALLOW_THREADS;
|
||||
_PySSL_FIX_ERRNO;
|
||||
self->err = err;
|
||||
|
||||
if (count < 0)
|
||||
|
|
@ -2617,10 +2616,11 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
|
|||
deadline = _PyDeadline_Init(timeout);
|
||||
|
||||
do {
|
||||
PySSL_BEGIN_ALLOW_THREADS(self)
|
||||
Py_BEGIN_ALLOW_THREADS;
|
||||
retval = SSL_read_ex(self->ssl, mem, (size_t)len, &count);
|
||||
err = _PySSL_errno(retval == 0, self->ssl, retval);
|
||||
PySSL_END_ALLOW_THREADS(self)
|
||||
Py_END_ALLOW_THREADS;
|
||||
_PySSL_FIX_ERRNO;
|
||||
self->err = err;
|
||||
|
||||
if (PyErr_CheckSignals())
|
||||
|
|
@ -2719,7 +2719,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
|
|||
}
|
||||
|
||||
while (1) {
|
||||
PySSL_BEGIN_ALLOW_THREADS(self)
|
||||
Py_BEGIN_ALLOW_THREADS;
|
||||
/* Disable read-ahead so that unwrap can work correctly.
|
||||
* Otherwise OpenSSL might read in too much data,
|
||||
* eating clear text data that happens to be
|
||||
|
|
@ -2732,7 +2732,8 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
|
|||
SSL_set_read_ahead(self->ssl, 0);
|
||||
ret = SSL_shutdown(self->ssl);
|
||||
err = _PySSL_errno(ret < 0, self->ssl, ret);
|
||||
PySSL_END_ALLOW_THREADS(self)
|
||||
Py_END_ALLOW_THREADS;
|
||||
_PySSL_FIX_ERRNO;
|
||||
self->err = err;
|
||||
|
||||
/* If err == 1, a secure shutdown with SSL_shutdown() is complete */
|
||||
|
|
|
|||
|
|
@ -135,7 +135,6 @@ _PySSL_keylog_callback(const SSL *ssl, const char *line)
|
|||
* critical debug helper.
|
||||
*/
|
||||
|
||||
assert(PyMutex_IsLocked(&ssl_obj->tstate_mutex));
|
||||
Py_BEGIN_ALLOW_THREADS
|
||||
PyThread_acquire_lock(lock, 1);
|
||||
res = BIO_printf(ssl_obj->ctx->keylog_bio, "%s\n", line);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue