Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc649df6

Browse files
authored
gh-104372: Cleanup _posixsubprocessmake_inheritable for async signal safety and no GIL requirement (#104518)
Move all of the Python C API calls into the parent process up frontinstead of doing PyLong_AsLong and PyErr_Occurred and PyTuple_GET fromthe post-fork/vfork child process.Much of this was long overdue. We shouldn't have been using PyTuple andPyLong APIs within all of these low level functions anyways.
1 parentf7df173 commitc649df6

File tree

2 files changed

+92
-34
lines changed

2 files changed

+92
-34
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Refactored the ``_posixsubprocess`` internals to avoid Python C API usage between fork and exec when marking ``pass_fds=`` file descriptors inheritable.

‎Modules/_posixsubprocess.c

Lines changed: 91 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,17 @@ _sanity_check_python_fd_sequence(PyObject *fd_sequence)
160160

161161
/* Is fd found in the sorted Python Sequence? */
162162
staticint
163-
_is_fd_in_sorted_fd_sequence(intfd,PyObject*fd_sequence)
163+
_is_fd_in_sorted_fd_sequence(intfd,int*fd_sequence,
164+
Py_ssize_tfd_sequence_len)
164165
{
165166
/* Binary search. */
166167
Py_ssize_tsearch_min=0;
167-
Py_ssize_tsearch_max=PyTuple_GET_SIZE(fd_sequence)-1;
168+
Py_ssize_tsearch_max=fd_sequence_len-1;
168169
if (search_max<0)
169170
return0;
170171
do {
171172
longmiddle= (search_min+search_max) /2;
172-
longmiddle_fd=PyLong_AsLong(PyTuple_GET_ITEM(fd_sequence,middle));
173+
longmiddle_fd=fd_sequence[middle];
173174
if (fd==middle_fd)
174175
return1;
175176
if (fd>middle_fd)
@@ -180,24 +181,56 @@ _is_fd_in_sorted_fd_sequence(int fd, PyObject *fd_sequence)
180181
return0;
181182
}
182183

184+
/*
185+
* Do all the Python C API calls in the parent process to turn the pass_fds
186+
* "py_fds_to_keep" tuple into a C array. The caller owns allocation and
187+
* freeing of the array.
188+
*
189+
* On error an unknown number of array elements may have been filled in.
190+
* A Python exception has been set when an error is returned.
191+
*
192+
* Returns: -1 on error, 0 on success.
193+
*/
183194
staticint
184-
make_inheritable(PyObject*py_fds_to_keep,interrpipe_write)
195+
convert_fds_to_keep_to_c(PyObject*py_fds_to_keep,int*c_fds_to_keep)
185196
{
186197
Py_ssize_ti,len;
187198

188199
len=PyTuple_GET_SIZE(py_fds_to_keep);
189200
for (i=0;i<len;++i) {
190201
PyObject*fdobj=PyTuple_GET_ITEM(py_fds_to_keep,i);
191202
longfd=PyLong_AsLong(fdobj);
192-
assert(!PyErr_Occurred());
193-
assert(0 <=fd&&fd <=INT_MAX);
203+
if (PyErr_Occurred()) {
204+
return-1;
205+
}
206+
if (fd<0||fd>INT_MAX) {
207+
PyErr_SetString(PyExc_ValueError,
208+
"fd out of range in fds_to_keep.");
209+
return-1;
210+
}
211+
c_fds_to_keep[i]= (int)fd;
212+
}
213+
return0;
214+
}
215+
216+
217+
/* This function must be async-signal-safe as it is called from child_exec()
218+
* after fork() or vfork().
219+
*/
220+
staticint
221+
make_inheritable(int*c_fds_to_keep,Py_ssize_tlen,interrpipe_write)
222+
{
223+
Py_ssize_ti;
224+
225+
for (i=0;i<len;++i) {
226+
intfd=c_fds_to_keep[i];
194227
if (fd==errpipe_write) {
195-
/* errpipe_write is part ofpy_fds_to_keep. It must be closed at
228+
/* errpipe_write is part offds_to_keep. It must be closed at
196229
exec(), but kept open in the child process until exec() is
197230
called. */
198231
continue;
199232
}
200-
if (_Py_set_inheritable_async_safe((int)fd,1,NULL)<0)
233+
if (_Py_set_inheritable_async_safe(fd,1,NULL)<0)
201234
return-1;
202235
}
203236
return0;
@@ -233,7 +266,7 @@ safe_get_max_fd(void)
233266

234267

235268
/* Close all file descriptors in the given range except for those in
236-
*py_fds_to_keep by invoking closer on each subrange.
269+
*fds_to_keep by invoking closer on each subrange.
237270
*
238271
* If end_fd == -1, it's guessed via safe_get_max_fd(), but it isn't
239272
* possible to know for sure what the max fd to go up to is for
@@ -243,19 +276,18 @@ safe_get_max_fd(void)
243276
staticint
244277
_close_range_except(intstart_fd,
245278
intend_fd,
246-
PyObject*py_fds_to_keep,
279+
int*fds_to_keep,
280+
Py_ssize_tfds_to_keep_len,
247281
int (*closer)(int,int))
248282
{
249283
if (end_fd==-1) {
250284
end_fd=Py_MIN(safe_get_max_fd(),INT_MAX);
251285
}
252-
Py_ssize_tnum_fds_to_keep=PyTuple_GET_SIZE(py_fds_to_keep);
253286
Py_ssize_tkeep_seq_idx;
254-
/* Aspy_fds_to_keep is sorted we can loop through the list closing
287+
/* Asfds_to_keep is sorted we can loop through the list closing
255288
* fds in between any in the keep list falling within our range. */
256-
for (keep_seq_idx=0;keep_seq_idx<num_fds_to_keep;++keep_seq_idx) {
257-
PyObject*py_keep_fd=PyTuple_GET_ITEM(py_fds_to_keep,keep_seq_idx);
258-
intkeep_fd=PyLong_AsLong(py_keep_fd);
289+
for (keep_seq_idx=0;keep_seq_idx<fds_to_keep_len;++keep_seq_idx) {
290+
intkeep_fd=fds_to_keep[keep_seq_idx];
259291
if (keep_fd<start_fd)
260292
continue;
261293
if (closer(start_fd,keep_fd-1)!=0)
@@ -295,7 +327,7 @@ _brute_force_closer(int first, int last)
295327
}
296328

297329
/* Close all open file descriptors in the range from start_fd and higher
298-
* Do not close any in the sortedpy_fds_to_keep list.
330+
* Do not close any in the sortedfds_to_keep list.
299331
*
300332
* This version is async signal safe as it does not make any unsafe C library
301333
* calls, malloc calls or handle any locks. It is _unfortunate_ to be forced
@@ -310,14 +342,16 @@ _brute_force_closer(int first, int last)
310342
* it with some cpp #define magic to work on other OSes as well if you want.
311343
*/
312344
staticvoid
313-
_close_open_fds_safe(intstart_fd,PyObject*py_fds_to_keep)
345+
_close_open_fds_safe(intstart_fd,int*fds_to_keep,Py_ssize_tfds_to_keep_len)
314346
{
315347
intfd_dir_fd;
316348

317349
fd_dir_fd=_Py_open_noraise(FD_DIR,O_RDONLY);
318350
if (fd_dir_fd==-1) {
319351
/* No way to get a list of open fds. */
320-
_close_range_except(start_fd,-1,py_fds_to_keep,_brute_force_closer);
352+
_close_range_except(start_fd,-1,
353+
fds_to_keep,fds_to_keep_len,
354+
_brute_force_closer);
321355
return;
322356
}else {
323357
charbuffer[sizeof(structlinux_dirent64)];
@@ -336,7 +370,8 @@ _close_open_fds_safe(int start_fd, PyObject* py_fds_to_keep)
336370
if ((fd=_pos_int_from_ascii(entry->d_name))<0)
337371
continue;/* Not a number. */
338372
if (fd!=fd_dir_fd&&fd >=start_fd&&
339-
!_is_fd_in_sorted_fd_sequence(fd,py_fds_to_keep)) {
373+
!_is_fd_in_sorted_fd_sequence(fd,fds_to_keep,
374+
fds_to_keep_len)) {
340375
close(fd);
341376
}
342377
}
@@ -357,7 +392,7 @@ _unsafe_closer(int first, int last)
357392
}
358393

359394
/* Close all open file descriptors from start_fd and higher.
360-
* Do not close any in the sortedpy_fds_to_keep tuple.
395+
* Do not close any in the sortedfds_to_keep tuple.
361396
*
362397
* This function violates the strict use of async signal safe functions. :(
363398
* It calls opendir(), readdir() and closedir(). Of these, the one most
@@ -370,11 +405,13 @@ _unsafe_closer(int first, int last)
370405
* http://womble.decadent.org.uk/readdir_r-advisory.html
371406
*/
372407
staticvoid
373-
_close_open_fds_maybe_unsafe(intstart_fd,PyObject*py_fds_to_keep)
408+
_close_open_fds_maybe_unsafe(intstart_fd,int*fds_to_keep,
409+
Py_ssize_tfds_to_keep_len)
374410
{
375411
DIR*proc_fd_dir;
376412
#ifndefHAVE_DIRFD
377-
while (_is_fd_in_sorted_fd_sequence(start_fd,py_fds_to_keep)) {
413+
while (_is_fd_in_sorted_fd_sequence(start_fd,fds_to_keep,
414+
fds_to_keep_len)) {
378415
++start_fd;
379416
}
380417
/* Close our lowest fd before we call opendir so that it is likely to
@@ -393,7 +430,8 @@ _close_open_fds_maybe_unsafe(int start_fd, PyObject* py_fds_to_keep)
393430
proc_fd_dir=opendir(FD_DIR);
394431
if (!proc_fd_dir) {
395432
/* No way to get a list of open fds. */
396-
_close_range_except(start_fd,-1,py_fds_to_keep,_unsafe_closer);
433+
_close_range_except(start_fd,-1,fds_to_keep,fds_to_keep_len,
434+
_unsafe_closer);
397435
}else {
398436
structdirent*dir_entry;
399437
#ifdefHAVE_DIRFD
@@ -407,14 +445,16 @@ _close_open_fds_maybe_unsafe(int start_fd, PyObject* py_fds_to_keep)
407445
if ((fd=_pos_int_from_ascii(dir_entry->d_name))<0)
408446
continue;/* Not a number. */
409447
if (fd!=fd_used_by_opendir&&fd >=start_fd&&
410-
!_is_fd_in_sorted_fd_sequence(fd,py_fds_to_keep)) {
448+
!_is_fd_in_sorted_fd_sequence(fd,fds_to_keep,
449+
fds_to_keep_len)) {
411450
close(fd);
412451
}
413452
errno=0;
414453
}
415454
if (errno) {
416455
/* readdir error, revert behavior. Highly Unlikely. */
417-
_close_range_except(start_fd,-1,py_fds_to_keep,_unsafe_closer);
456+
_close_range_except(start_fd,-1,fds_to_keep,fds_to_keep_len,
457+
_unsafe_closer);
418458
}
419459
closedir(proc_fd_dir);
420460
}
@@ -442,16 +482,16 @@ _close_range_closer(int first, int last)
442482
#endif
443483

444484
staticvoid
445-
_close_open_fds(intstart_fd,PyObject*py_fds_to_keep)
485+
_close_open_fds(intstart_fd,int*fds_to_keep,Py_ssize_tfds_to_keep_len)
446486
{
447487
#ifdefHAVE_ASYNC_SAFE_CLOSE_RANGE
448488
if (_close_range_except(
449-
start_fd,INT_MAX,py_fds_to_keep,
489+
start_fd,INT_MAX,fds_to_keep,fds_to_keep_len,
450490
_close_range_closer)==0) {
451491
return;
452492
}
453493
#endif
454-
_close_open_fds_fallback(start_fd,py_fds_to_keep);
494+
_close_open_fds_fallback(start_fd,fds_to_keep,fds_to_keep_len);
455495
}
456496

457497
#ifdefVFORK_USABLE
@@ -544,7 +584,7 @@ child_exec(char *const exec_array[],
544584
Py_ssize_textra_group_size,constgid_t*extra_groups,
545585
uid_tuid,intchild_umask,
546586
constvoid*child_sigmask,
547-
PyObject*py_fds_to_keep,
587+
int*fds_to_keep,Py_ssize_tfds_to_keep_len,
548588
PyObject*preexec_fn,
549589
PyObject*preexec_fn_args_tuple)
550590
{
@@ -554,7 +594,7 @@ child_exec(char *const exec_array[],
554594
/* Buffer large enough to hold a hex integer. We can't malloc. */
555595
charhex_errno[sizeof(saved_errno)*2+1];
556596

557-
if (make_inheritable(py_fds_to_keep,errpipe_write)<0)
597+
if (make_inheritable(fds_to_keep,fds_to_keep_len,errpipe_write)<0)
558598
gotoerror;
559599

560600
/* Close parent's pipe ends. */
@@ -676,7 +716,7 @@ child_exec(char *const exec_array[],
676716
/* close FDs after executing preexec_fn, which might open FDs */
677717
if (close_fds) {
678718
/* TODO HP-UX could use pstat_getproc() if anyone cares about it. */
679-
_close_open_fds(3,py_fds_to_keep);
719+
_close_open_fds(3,fds_to_keep,fds_to_keep_len);
680720
}
681721

682722
/* This loop matches the Lib/os.py _execvpe()'s PATH search when */
@@ -750,7 +790,7 @@ do_fork_exec(char *const exec_array[],
750790
Py_ssize_textra_group_size,constgid_t*extra_groups,
751791
uid_tuid,intchild_umask,
752792
constvoid*child_sigmask,
753-
PyObject*py_fds_to_keep,
793+
int*fds_to_keep,Py_ssize_tfds_to_keep_len,
754794
PyObject*preexec_fn,
755795
PyObject*preexec_fn_args_tuple)
756796
{
@@ -801,7 +841,8 @@ do_fork_exec(char *const exec_array[],
801841
close_fds,restore_signals,call_setsid,pgid_to_set,
802842
gid,extra_group_size,extra_groups,
803843
uid,child_umask,child_sigmask,
804-
py_fds_to_keep,preexec_fn,preexec_fn_args_tuple);
844+
fds_to_keep,fds_to_keep_len,
845+
preexec_fn,preexec_fn_args_tuple);
805846
_exit(255);
806847
return0;/* Dead code to avoid a potential compiler warning. */
807848
}
@@ -881,6 +922,8 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
881922
Py_ssize_textra_group_size=0;
882923
intneed_after_fork=0;
883924
intsaved_errno=0;
925+
int*c_fds_to_keep=NULL;
926+
Py_ssize_tfds_to_keep_len=PyTuple_GET_SIZE(py_fds_to_keep);
884927

885928
PyInterpreterState*interp=PyInterpreterState_Get();
886929
if ((preexec_fn!=Py_None)&& (interp!=PyInterpreterState_Main())) {
@@ -1031,6 +1074,15 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
10311074
#endif/* HAVE_SETREUID */
10321075
}
10331076

1077+
c_fds_to_keep=PyMem_RawMalloc(fds_to_keep_len*sizeof(int));
1078+
if (c_fds_to_keep==NULL) {
1079+
PyErr_SetString(PyExc_MemoryError,"failed to malloc c_fds_to_keep");
1080+
gotocleanup;
1081+
}
1082+
if (convert_fds_to_keep_to_c(py_fds_to_keep,c_fds_to_keep)<0) {
1083+
gotocleanup;
1084+
}
1085+
10341086
/* This must be the last thing done before fork() because we do not
10351087
* want to call PyOS_BeforeFork() if there is any chance of another
10361088
* error leading to the cleanup: code without calling fork(). */
@@ -1073,7 +1125,8 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
10731125
close_fds,restore_signals,call_setsid,pgid_to_set,
10741126
gid,extra_group_size,extra_groups,
10751127
uid,child_umask,old_sigmask,
1076-
py_fds_to_keep,preexec_fn,preexec_fn_args_tuple);
1128+
c_fds_to_keep,fds_to_keep_len,
1129+
preexec_fn,preexec_fn_args_tuple);
10771130

10781131
/* Parent (original) process */
10791132
if (pid== (pid_t)-1) {
@@ -1103,6 +1156,10 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
11031156
PyOS_AfterFork_Parent();
11041157

11051158
cleanup:
1159+
if (c_fds_to_keep!=NULL) {
1160+
PyMem_RawFree(c_fds_to_keep);
1161+
}
1162+
11061163
if (saved_errno!=0) {
11071164
errno=saved_errno;
11081165
/* We can't call this above as PyOS_AfterFork_Parent() calls back

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp