Skip to content

Commit

Permalink
gh-128277: make globals variables thread safe in socket module (#128286)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kumaraditya303 @WolframAlph
kumaraditya303 authored and WolframAlph committed Dec 31, 2024
1 parent b2ac70a commit 51a2015
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 36 deletions.
5 changes: 5 additions & 0 deletions Lib/test/test_cmd_line.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,9 @@ def test_osx_android_utf8(self):
self.assertEqual(p.returncode, 0)

def test_non_interactive_output_buffering(self):
PYTHONUNBUFFERED_ENV_VAR = os.environ["PYTHONUNBUFFERED"]
# we expect buffered stdio
os.environ["PYTHONUNBUFFERED"] = "0"
code = textwrap.dedent("""
import sys
out = sys.stdout
Expand All @@ -350,6 +353,8 @@ def test_non_interactive_output_buffering(self):
self.assertEqual(proc.stdout,
'False False False\n'
'False False True\n')
# restore original value
os.environ["PYTHONUNBUFFERED"] = PYTHONUNBUFFERED_ENV_VAR

def test_unbuffered_output(self):
# Test expected operation of the '-u' switch
Expand Down
56 changes: 23 additions & 33 deletions Modules/socketmodule.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,20 +550,20 @@ typedef struct _socket_state {

/* Default timeout for new sockets */
PyTime_t defaulttimeout;
} socket_state;

#if defined(HAVE_ACCEPT) || defined(HAVE_ACCEPT4)
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
/* accept4() is available on Linux 2.6.28+ and glibc 2.10 */
int accept4_works;
/* accept4() is available on Linux 2.6.28+ and glibc 2.10 */
static int accept4_works = -1;
#endif
#endif

#ifdef SOCK_CLOEXEC
/* socket() and socketpair() fail with EINVAL on Linux kernel older
* than 2.6.27 if SOCK_CLOEXEC flag is set in the socket type. */
int sock_cloexec_works;
/* socket() and socketpair() fail with EINVAL on Linux kernel older
* than 2.6.27 if SOCK_CLOEXEC flag is set in the socket type. */
static int sock_cloexec_works = -1;
#endif
} socket_state;

static inline void
set_sock_fd(PySocketSockObject *s, SOCKET_T fd)
Expand Down Expand Up @@ -2904,16 +2904,15 @@ sock_accept_impl(PySocketSockObject *s, void *data)
#endif

#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
socket_state *state = s->state;
if (state->accept4_works != 0) {
if (_Py_atomic_load_int_relaxed(&accept4_works) != 0) {
ctx->result = accept4(get_sock_fd(s), addr, paddrlen,
SOCK_CLOEXEC);
if (ctx->result == INVALID_SOCKET && state->accept4_works == -1) {
if (ctx->result == INVALID_SOCKET && _Py_atomic_load_int_relaxed(&accept4_works) == -1) {
/* On Linux older than 2.6.28, accept4() fails with ENOSYS */
state->accept4_works = (errno != ENOSYS);
_Py_atomic_store_int_relaxed(&accept4_works, errno != ENOSYS);
}
}
if (state->accept4_works == 0)
if (_Py_atomic_load_int_relaxed(&accept4_works) == 0)
ctx->result = accept(get_sock_fd(s), addr, paddrlen);
#else
ctx->result = accept(get_sock_fd(s), addr, paddrlen);
Expand Down Expand Up @@ -2966,8 +2965,7 @@ sock_accept(PySocketSockObject *s, PyObject *Py_UNUSED(ignored))
#else

#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
socket_state *state = s->state;
if (!state->accept4_works)
if (!_Py_atomic_load_int_relaxed(&accept4_works))
#endif
{
if (_Py_set_inheritable(newfd, 0, NULL) < 0) {
Expand Down Expand Up @@ -5428,7 +5426,7 @@ sock_initobj_impl(PySocketSockObject *self, int family, int type, int proto,

#ifndef MS_WINDOWS
#ifdef SOCK_CLOEXEC
int *atomic_flag_works = &state->sock_cloexec_works;
int *atomic_flag_works = &sock_cloexec_works;
#else
int *atomic_flag_works = NULL;
#endif
Expand Down Expand Up @@ -5583,15 +5581,16 @@ sock_initobj_impl(PySocketSockObject *self, int family, int type, int proto,
/* UNIX */
Py_BEGIN_ALLOW_THREADS
#ifdef SOCK_CLOEXEC
if (state->sock_cloexec_works != 0) {
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) != 0) {
fd = socket(family, type | SOCK_CLOEXEC, proto);
if (state->sock_cloexec_works == -1) {
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) == -1) {
if (fd >= 0) {
state->sock_cloexec_works = 1;
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 1);
}

else if (errno == EINVAL) {
/* Linux older than 2.6.27 does not support SOCK_CLOEXEC */
state->sock_cloexec_works = 0;
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 0);
fd = socket(family, type, proto);
}
}
Expand Down Expand Up @@ -6332,7 +6331,7 @@ socket_socketpair(PyObject *self, PyObject *args)
PyObject *res = NULL;
socket_state *state = get_module_state(self);
#ifdef SOCK_CLOEXEC
int *atomic_flag_works = &state->sock_cloexec_works;
int *atomic_flag_works = &sock_cloexec_works;
#else
int *atomic_flag_works = NULL;
#endif
Expand All @@ -6350,15 +6349,15 @@ socket_socketpair(PyObject *self, PyObject *args)
/* Create a pair of socket fds */
Py_BEGIN_ALLOW_THREADS
#ifdef SOCK_CLOEXEC
if (state->sock_cloexec_works != 0) {
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) != 0) {
ret = socketpair(family, type | SOCK_CLOEXEC, proto, sv);
if (state->sock_cloexec_works == -1) {
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) == -1) {
if (ret >= 0) {
state->sock_cloexec_works = 1;
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 1);
}
else if (errno == EINVAL) {
/* Linux older than 2.6.27 does not support SOCK_CLOEXEC */
state->sock_cloexec_works = 0;
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 0);
ret = socketpair(family, type, proto, sv);
}
}
Expand Down Expand Up @@ -7466,17 +7465,8 @@ socket_exec(PyObject *m)
}

socket_state *state = get_module_state(m);
state->defaulttimeout = _PYTIME_FROMSECONDS(-1);

#if defined(HAVE_ACCEPT) || defined(HAVE_ACCEPT4)
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
state->accept4_works = -1;
#endif
#endif

#ifdef SOCK_CLOEXEC
state->sock_cloexec_works = -1;
#endif
_Py_atomic_store_int64_relaxed(&state->defaulttimeout, _PYTIME_FROMSECONDS(-1));

#define ADD_EXC(MOD, NAME, VAR, BASE) do { \
VAR = PyErr_NewException("socket." NAME, BASE, NULL); \
Expand Down
6 changes: 3 additions & 3 deletions Python/fileutils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1468,14 +1468,14 @@ set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works)
assert(!(atomic_flag_works != NULL && inheritable));

if (atomic_flag_works != NULL && !inheritable) {
if (*atomic_flag_works == -1) {
if (_Py_atomic_load_int_relaxed(atomic_flag_works) == -1) {
int isInheritable = get_inheritable(fd, raise);
if (isInheritable == -1)
return -1;
*atomic_flag_works = !isInheritable;
_Py_atomic_store_int_relaxed(atomic_flag_works, !isInheritable);
}

if (*atomic_flag_works)
if (_Py_atomic_load_int_relaxed(atomic_flag_works))
return 0;
}

Expand Down
2 changes: 2 additions & 0 deletions Tools/c-analyzer/cpython/globals-to-fix.tsv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,3 +444,5 @@ Modules/readline.c - completed_input_string -
Modules/rotatingtree.c - random_stream -
Modules/rotatingtree.c - random_value -
Modules/rotatingtree.c - random_mutex -
Modules/socketmodule.c - accept4_works -
Modules/socketmodule.c - sock_cloexec_works -

0 comments on commit 51a2015

Please sign in to comment.