Skip to content

Commit e389d6c

Browse files
kumaraditya303kumaraditya303
authored
gh-128277: make globals variables thread safe in socket module (#128286)
1 parent b2ac70a commit e389d6c

File tree

3 files changed

+28
-36
lines changed

3 files changed

+28
-36
lines changed

‎Modules/socketmodule.c

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -550,20 +550,20 @@ typedef struct _socket_state {
550550

551551
/* Default timeout for new sockets */
552552
PyTime_t defaulttimeout;
553+
} socket_state;
553554

554555
#if defined(HAVE_ACCEPT) || defined(HAVE_ACCEPT4)
555556
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
556-
/* accept4() is available on Linux 2.6.28+ and glibc 2.10 */
557-
int accept4_works;
557+
/* accept4() is available on Linux 2.6.28+ and glibc 2.10 */
558+
static int accept4_works = -1;
558559
#endif
559560
#endif
560561

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

568568
static inline void
569569
set_sock_fd(PySocketSockObject *s, SOCKET_T fd)
@@ -2904,16 +2904,15 @@ sock_accept_impl(PySocketSockObject *s, void *data)
29042904
#endif
29052905

29062906
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
2907-
socket_state *state = s->state;
2908-
if (state->accept4_works != 0) {
2907+
if (_Py_atomic_load_int_relaxed(&accept4_works) != 0) {
29092908
ctx->result = accept4(get_sock_fd(s), addr, paddrlen,
29102909
SOCK_CLOEXEC);
2911-
if (ctx->result == INVALID_SOCKET && state->accept4_works == -1) {
2910+
if (ctx->result == INVALID_SOCKET && _Py_atomic_load_int_relaxed(&accept4_works) == -1) {
29122911
/* On Linux older than 2.6.28, accept4() fails with ENOSYS */
2913-
state->accept4_works = (errno != ENOSYS);
2912+
_Py_atomic_store_int_relaxed(&accept4_works, errno != ENOSYS);
29142913
}
29152914
}
2916-
if (state->accept4_works == 0)
2915+
if (_Py_atomic_load_int_relaxed(&accept4_works) == 0)
29172916
ctx->result = accept(get_sock_fd(s), addr, paddrlen);
29182917
#else
29192918
ctx->result = accept(get_sock_fd(s), addr, paddrlen);
@@ -2966,8 +2965,7 @@ sock_accept(PySocketSockObject *s, PyObject *Py_UNUSED(ignored))
29662965
#else
29672966

29682967
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
2969-
socket_state *state = s->state;
2970-
if (!state->accept4_works)
2968+
if (!_Py_atomic_load_int_relaxed(&accept4_works))
29712969
#endif
29722970
{
29732971
if (_Py_set_inheritable(newfd, 0, NULL) < 0) {
@@ -5428,7 +5426,7 @@ sock_initobj_impl(PySocketSockObject *self, int family, int type, int proto,
54285426

54295427
#ifndef MS_WINDOWS
54305428
#ifdef SOCK_CLOEXEC
5431-
int *atomic_flag_works = &state->sock_cloexec_works;
5429+
int *atomic_flag_works = &sock_cloexec_works;
54325430
#else
54335431
int *atomic_flag_works = NULL;
54345432
#endif
@@ -5583,15 +5581,16 @@ sock_initobj_impl(PySocketSockObject *self, int family, int type, int proto,
55835581
/* UNIX */
55845582
Py_BEGIN_ALLOW_THREADS
55855583
#ifdef SOCK_CLOEXEC
5586-
if (state->sock_cloexec_works != 0) {
5584+
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) != 0) {
55875585
fd = socket(family, type | SOCK_CLOEXEC, proto);
5588-
if (state->sock_cloexec_works == -1) {
5586+
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) == -1) {
55895587
if (fd >= 0) {
5590-
state->sock_cloexec_works = 1;
5588+
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 1);
55915589
}
5590+
55925591
else if (errno == EINVAL) {
55935592
/* Linux older than 2.6.27 does not support SOCK_CLOEXEC */
5594-
state->sock_cloexec_works = 0;
5593+
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 0);
55955594
fd = socket(family, type, proto);
55965595
}
55975596
}
@@ -6332,7 +6331,7 @@ socket_socketpair(PyObject *self, PyObject *args)
63326331
PyObject *res = NULL;
63336332
socket_state *state = get_module_state(self);
63346333
#ifdef SOCK_CLOEXEC
6335-
int *atomic_flag_works = &state->sock_cloexec_works;
6334+
int *atomic_flag_works = &sock_cloexec_works;
63366335
#else
63376336
int *atomic_flag_works = NULL;
63386337
#endif
@@ -6350,15 +6349,15 @@ socket_socketpair(PyObject *self, PyObject *args)
63506349
/* Create a pair of socket fds */
63516350
Py_BEGIN_ALLOW_THREADS
63526351
#ifdef SOCK_CLOEXEC
6353-
if (state->sock_cloexec_works != 0) {
6352+
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) != 0) {
63546353
ret = socketpair(family, type | SOCK_CLOEXEC, proto, sv);
6355-
if (state->sock_cloexec_works == -1) {
6354+
if (_Py_atomic_load_int_relaxed(&sock_cloexec_works) == -1) {
63566355
if (ret >= 0) {
6357-
state->sock_cloexec_works = 1;
6356+
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 1);
63586357
}
63596358
else if (errno == EINVAL) {
63606359
/* Linux older than 2.6.27 does not support SOCK_CLOEXEC */
6361-
state->sock_cloexec_works = 0;
6360+
_Py_atomic_store_int_relaxed(&sock_cloexec_works, 0);
63626361
ret = socketpair(family, type, proto, sv);
63636362
}
63646363
}
@@ -7466,17 +7465,8 @@ socket_exec(PyObject *m)
74667465
}
74677466

74687467
socket_state *state = get_module_state(m);
7469-
state->defaulttimeout = _PYTIME_FROMSECONDS(-1);
7470-
7471-
#if defined(HAVE_ACCEPT) || defined(HAVE_ACCEPT4)
7472-
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
7473-
state->accept4_works = -1;
7474-
#endif
7475-
#endif
74767468

7477-
#ifdef SOCK_CLOEXEC
7478-
state->sock_cloexec_works = -1;
7479-
#endif
7469+
_Py_atomic_store_int64_relaxed(&state->defaulttimeout, _PYTIME_FROMSECONDS(-1));
74807470

74817471
#define ADD_EXC(MOD, NAME, VAR, BASE) do { \
74827472
VAR = PyErr_NewException("socket." NAME, BASE, NULL); \

‎Python/fileutils.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,14 +1468,14 @@ set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works)
14681468
assert(!(atomic_flag_works != NULL && inheritable));
14691469

14701470
if (atomic_flag_works != NULL && !inheritable) {
1471-
if (*atomic_flag_works == -1) {
1471+
if (_Py_atomic_load_int_relaxed(atomic_flag_works) == -1) {
14721472
int isInheritable = get_inheritable(fd, raise);
14731473
if (isInheritable == -1)
14741474
return -1;
1475-
*atomic_flag_works = !isInheritable;
1475+
_Py_atomic_store_int_relaxed(atomic_flag_works, !isInheritable);
14761476
}
14771477

1478-
if (*atomic_flag_works)
1478+
if (_Py_atomic_load_int_relaxed(atomic_flag_works))
14791479
return 0;
14801480
}
14811481

‎Tools/c-analyzer/cpython/globals-to-fix.tsv

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,3 +444,5 @@ Modules/readline.c - completed_input_string -
444444
Modules/rotatingtree.c - random_stream -
445445
Modules/rotatingtree.c - random_value -
446446
Modules/rotatingtree.c - random_mutex -
447+
Modules/socketmodule.c - accept4_works -
448+
Modules/socketmodule.c - sock_cloexec_works -

0 commit comments

Comments
 (0)