8d8fabee-d1ef-4079-82b7-73db0595456d
commented
10 years ago Hello,
I noticed some possible bad behaviour while working on Python issue 21618 (see http://bugs.python.org/issue21618). Python has the following code in _posixsubmodules.c for closing extra files before spawning a process:
static void
_close_open_fd_range_safe(int start_fd, int end_fd, PyObject* py_fds_to_keep)
{
int fd_dir_fd;
if (start_fd >= end_fd)
return;
fd_dir_fd = _Py_open(FD_DIR, O_RDONLY);
if (fd_dir_fd == -1) {
/* No way to get a list of open fds. */
_close_fds_by_brute_force(start_fd, end_fd, py_fds_to_keep);
return;
} else {
char buffer[sizeof(struct linux_dirent64)];
int bytes;
while ((bytes = syscall(SYS_getdents64, fd_dir_fd,
(struct linux_dirent64 *)buffer,
sizeof(buffer))) > 0) {
struct linux_dirent64 *entry;
int offset;
for (offset = 0; offset < bytes; offset += entry->d_reclen) {
int fd;
entry = (struct linux_dirent64 *)(buffer + offset);
if ((fd = _pos_int_from_ascii(entry->d_name)) < 0)
continue; /* Not a number. */
if (fd != fd_dir_fd && fd >= start_fd && fd < end_fd &&
!_is_fd_in_sorted_fd_sequence(fd, py_fds_to_keep)) {
while (close(fd) < 0 && errno == EINTR);
}
}
}
close(fd_dir_fd);
}
}In the code FD_DIR is "/proc/self/fd" on Linux. I'm not sure this code is correct. This seems as if it would have the same problems as iterating over a list and modifying it at the same time.
I can think of a few solutions but they all have problems.
One could allocate a list of open files once and then iterate through that list and close the files but this is not signal safe so this solution is incorrect. One possible workaround is to use mmap to allocate the memory. This is a direct system call and I don't see a reason for it not to be signal safe but I'm not sure. One neat hack would be too mmap the memory ahead of time and then rely on lazy paging to allocate the memory lazily. Another solution is to search for the largest open file and then iterate over and close all the possible file descriptors in between. So far most possible solutions just seem really hacky to me though.
I feel the best solution is to let the OS close the files and to set the O_CLOEXEC flag on the files that need to be closed.
vstinner
iritkatriel
serhiy-storchaka
gpshead
df79943f-4aee-4531-a00d-c6b12816eb70
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields: ```python assignee = 'https://github.com/gpshead' closed_at =
created_at =
labels = ['invalid', 'type-bug', 'library', '3.9', '3.10']
title = 'Concurrently closing files and iterating over the open files directory is not well specified'
updated_at =
user = 'https://bugs.python.org/sstewartgallus'
```
bugs.python.org fields:
```python
activity =
actor = 'vstinner'
assignee = 'gregory.p.smith'
closed = True
closed_date =
closer = 'vstinner'
components = ['Library (Lib)']
creation =
creator = 'sstewartgallus'
dependencies = []
files = ['35441', '35600']
hgrepos = []
issue_num = 21627
keywords = ['patch']
message_count = 17.0
messages = ['219510', '219541', '219542', '219579', '219582', '219651', '219773', '219908', '219922', '220376', '382586', '382603', '382618', '382724', '402073', '402208', '402212']
nosy_count = 8.0
nosy_names = ['gregory.p.smith', 'pitrou', 'vstinner', 'neologix', 'serhiy.storchaka', 'sstewartgallus', 'bup', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21627'
versions = ['Python 3.9', 'Python 3.10']
```