Skip to content

gh-89977: Avoid releasing the GIL in nonblocking socket operations #29579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jcrist wants to merge 4 commits into
base: main
Choose a base branch
from
+33 −19
Open

gh-89977: Avoid releasing the GIL in nonblocking socket operations #29579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4c77408
Don't release GIL for non-blocking syscalls
jcrist Nov 15, 2021
e313019
Skip releasing the GIL in nonblocking socket.close
jcrist Nov 16, 2021
196f9e6
Update Include/ceval.h
methane Feb 2, 2022
698eee2
Merge branch 'main' into keep-gil-for-fast-syscalls
methane Sep 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Include/ceval.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,20 @@ PyAPI_FUNC(void) PyEval_ReleaseThread(PyThreadState *tstate);
#define Py_END_ALLOW_THREADS PyEval_RestoreThread(_save); \
}


/* Conditionally release and restore the GIL. */
#define _Py_BEGIN_ALLOW_THREADS_COND(cond) \

@jcrist jcrist Nov 16, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some helper macros here, but am not sure if that's best practice. Happy to change this if others have a better suggestion.

{ \
PyThreadState *_save = NULL; \
if (cond) { \
_save = PyEval_SaveThread(); \
}
#define _Py_END_ALLOW_THREADS_COND \
if (_save != NULL) { \
PyEval_RestoreThread(_save); \
} \
}

/* Masks and values used by FORMAT_VALUE opcode. */
#define FVC_MASK 0x3
#define FVC_NONE 0x0
Expand Down
28 changes: 14 additions & 14 deletions Modules/selectmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,16 @@
}

do {
Py_BEGIN_ALLOW_THREADS
int timeout_is_zero = (tvp != NULL && tvp->tv_sec == 0 && tvp->tv_usec == 0);
_Py_BEGIN_ALLOW_THREADS_COND(!timeout_is_zero)
errno = 0;
n = select(
max,
imax ? &ifdset : NULL,
omax ? &ofdset : NULL,
emax ? &efdset : NULL,
tvp);
Py_END_ALLOW_THREADS
_Py_END_ALLOW_THREADS_COND

if (errno != EINTR)
break;
Expand Down Expand Up @@ -698,14 +699,14 @@
/* call poll() */
async_err = 0;
do {
Py_BEGIN_ALLOW_THREADS
_Py_BEGIN_ALLOW_THREADS_COND(ms != 0)

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Cross build Linux 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Address sanitizer (ubuntu-26.04) 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Hypothesis tests on Ubuntu 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Ubuntu (installed) / build, install and test 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Ubuntu (free-threading) / build and test (ubuntu-26.04-arm) 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Ubuntu / build and test (ubuntu-26.04) 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Ubuntu / build and test (ubuntu-26.04-arm) 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Ubuntu (free-threading) / build and test (ubuntu-26.04) 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?

Check failure on line 702 in Modules/selectmodule.c

View workflow job for this annotation

GitHub Actions / Ubuntu (bolt) / build and test (ubuntu-26.04) 

‘ms’ undeclared (first use in this function); did you mean ‘ts’?
errno = 0;
#ifdef HAVE_PPOLL
poll_result = ppoll(self->ufds, self->ufd_len, ts_p, NULL);
#else
poll_result = poll(self->ufds, self->ufd_len, (int)ms);
#endif
Py_END_ALLOW_THREADS
_Py_END_ALLOW_THREADS_COND

if (errno != EINTR)
break;
Expand Down Expand Up @@ -1034,10 +1035,10 @@

do {
/* call devpoll() */
Py_BEGIN_ALLOW_THREADS
_Py_BEGIN_ALLOW_THREADS_COND(ms != 0)
errno = 0;
poll_result = ioctl(self->fd_devpoll, DP_POLL, &dvp);
Py_END_ALLOW_THREADS
_Py_END_ALLOW_THREADS_COND

if (errno != EINTR)
break;
Expand Down Expand Up @@ -1513,17 +1514,13 @@
case EPOLL_CTL_MOD:
ev.events = events;
ev.data.fd = fd;
Py_BEGIN_ALLOW_THREADS
result = epoll_ctl(epfd, op, fd, &ev);
Py_END_ALLOW_THREADS

@jcrist jcrist Nov 16, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell epoll_ctl operations like this always complete quickly, so releasing the GIL here isn't really necessary.

break;
case EPOLL_CTL_DEL:
/* In kernel versions before 2.6.9, the EPOLL_CTL_DEL
* operation required a non-NULL pointer in event, even
* though this argument is ignored. */
Py_BEGIN_ALLOW_THREADS
result = epoll_ctl(epfd, op, fd, &ev);
Py_END_ALLOW_THREADS
break;
default:
result = -1;
Expand Down Expand Up @@ -1673,10 +1670,10 @@
}

do {
Py_BEGIN_ALLOW_THREADS
_Py_BEGIN_ALLOW_THREADS_COND(ms != 0)
errno = 0;
nfds = epoll_wait(self->epfd, evs, maxevents, (int)ms);
Py_END_ALLOW_THREADS
_Py_END_ALLOW_THREADS_COND

if (errno != EINTR)
break;
Expand Down Expand Up @@ -2429,11 +2426,14 @@
}

do {
Py_BEGIN_ALLOW_THREADS
int timeout_is_zero = (ptimeoutspec != NULL &&
ptimeoutspec->tv_sec == 0 &&
ptimeoutspec->tv_nsec == 0);
_Py_BEGIN_ALLOW_THREADS_COND(!timeout_is_zero)
errno = 0;
gotevents = kevent(self->kqfd, chl, nchanges,
evl, maxevents, ptimeoutspec);
Py_END_ALLOW_THREADS
_Py_END_ALLOW_THREADS_COND

if (errno != EINTR)
break;
Expand Down
10 changes: 5 additions & 5 deletions Modules/socketmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ internal_select(PySocketSockObject *s, int writing, PyTime_t interval,
When the function is retried, recompute the timeout using a monotonic clock.

sock_call_ex() must be called with the GIL held. The socket function is
called with the GIL released. */
called with the GIL released if the socket is blocking. */
static int
sock_call_ex(PySocketSockObject *s,
int writing,
Expand Down Expand Up @@ -1059,9 +1059,9 @@ sock_call_ex(PySocketSockObject *s,
/* inner loop to retry sock_func() when sock_func() is interrupted
by a signal */
while (1) {
Py_BEGIN_ALLOW_THREADS
_Py_BEGIN_ALLOW_THREADS_COND(s->sock_timeout)
res = sock_func(s, data);
Py_END_ALLOW_THREADS
_Py_END_ALLOW_THREADS_COND

@jcrist jcrist Nov 16, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are likely other socket operations where releasing the GIL could be avoided, but this gets the bulk of those done frequently in a loop (send/ recv, ...). socket.close() is also handled below.


if (res) {
/* sock_func() succeeded */
Expand Down Expand Up @@ -3655,9 +3655,9 @@ _socket_socket_close_impl(PySocketSockObject *s)
http://lwn.net/Articles/576478/ and
http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html
for more details. */
Py_BEGIN_ALLOW_THREADS
_Py_BEGIN_ALLOW_THREADS_COND(s->sock_timeout)
res = SOCKETCLOSE(fd);
Py_END_ALLOW_THREADS
_Py_END_ALLOW_THREADS_COND
Comment on lines -3658 to +3660

@methane methane Feb 2, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change here. s->sock_timeout doesn't affect to close.
Unlike send/recv, close can not be called frequently more than creating socket. So this shouldn't be a big performance problem.

/* bpo-30319: The peer can already have closed the connection.
Python ignores ECONNRESET on close(). */
if (res < 0 && errno != ECONNRESET) {
Expand Down
Loading