+2022-09-09 Bruno Haible <bruno@clisp.org>
+
+ spawn-pipe: Fix pipe-filter-* test hangs (regression 2020-12-24).
+ * lib/windows-spawn.h (struct inheritable_handles): Widen the per-fd
+ flags from 8 bits to 16 bits.
+ (KEEP_OPEN_IN_CHILD): New macro.
+ (init_inheritable_handles): Change description of what it does when
+ duplicate == true.
+ * lib/windows-spawn.c (init_inheritable_handles): If duplicate == true,
+ add all fds to the array, regardless whether they are scheduled to be
+ preserved in the child process.
+ (compose_handles_block): Update.
+ (spawnpvech): Update.
+ * lib/spawni.c (grow_inheritable_handles): Update.
+ (shrink_inheritable_handles): Also close the handles not marked with
+ KEEP_OPEN_IN_CHILD.
+ (do_open, do_dup2): Mark the new fd with KEEP_OPEN_IN_CHILD.
+
2022-09-09 Bruno Haible <bruno@clisp.org>
spawn-pipe: Fix possible hangs in programs that spawn several children.
errno = ENOMEM;
return -1;
}
- unsigned char *new_flags_array =
- (unsigned char *)
- realloc (inh_handles->flags, new_allocated * sizeof (unsigned char));
+ unsigned short *new_flags_array =
+ (unsigned short *)
+ realloc (inh_handles->flags, new_allocated * sizeof (unsigned short));
if (new_flags_array == NULL)
{
free (new_handles_array);
return 0;
}
-/* Reduces inh_handles->count to the minimum needed. */
+/* Closes the handles in inh_handles that are not meant to be preserved in the
+ child process, and reduces inh_handles->count to the minimum needed. */
static void
shrink_inheritable_handles (struct inheritable_handles *inh_handles)
{
HANDLE *handles = inh_handles->handles;
+ unsigned short *flags = inh_handles->flags;
+ size_t handles_count = inh_handles->count;
+ unsigned int fd;
+
+ for (fd = 0; fd < handles_count; fd++)
+ {
+ HANDLE handle = handles[fd];
+
+ if (handle != INVALID_HANDLE_VALUE
+ && (flags[fd] & KEEP_OPEN_IN_CHILD) == 0)
+ {
+ CloseHandle (handle);
+ handles[fd] = INVALID_HANDLE_VALUE;
+ }
+ }
+
+ while (handles_count > 3
+ && handles[handles_count - 1] == INVALID_HANDLE_VALUE)
+ handles_count--;
- while (inh_handles->count > 3
- && handles[inh_handles->count - 1] == INVALID_HANDLE_VALUE)
- inh_handles->count--;
+ inh_handles->count = handles_count;
}
/* Closes all handles in inh_handles. */
errno = EBADF; /* arbitrary */
return -1;
}
- inh_handles->flags[newfd] = ((flags & O_APPEND) != 0 ? 32 : 0);
+ inh_handles->flags[newfd] =
+ ((flags & O_APPEND) != 0 ? 32 : 0) | KEEP_OPEN_IN_CHILD;
return 0;
}
errno = EIO;
return -1;
}
- /* Duplicate the handle, so that it a forthcoming do_close action on oldfd
+ /* Duplicate the handle, so that a forthcoming do_close action on oldfd
has no effect on newfd. */
if (!DuplicateHandle (curr_process, inh_handles->handles[oldfd],
curr_process, &inh_handles->handles[newfd],
errno = EBADF; /* arbitrary */
return -1;
}
- inh_handles->flags[newfd] = 0;
+ inh_handles->flags[newfd] = KEEP_OPEN_IN_CHILD;
}
return 0;
}
}
}
- /* Reduce inh_handles.count to the minimum needed. */
+ /* Close the handles in inh_handles that are not meant to be preserved in the
+ child process, and reduce inh_handles.count to the minimum needed. */
shrink_inheritable_handles (&inh_handles);
/* CreateProcess
HANDLE handle = (HANDLE) _get_osfhandle (fd);
if (handle != INVALID_HANDLE_VALUE)
{
- DWORD hflags;
- /* GetHandleInformation
- <https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-gethandleinformation> */
- if (GetHandleInformation (handle, &hflags))
+ if (duplicate)
+ /* We will add fd to the array, regardless of whether it is
+ inheritable or not. */
+ break;
+ else
{
- if ((hflags & HANDLE_FLAG_INHERIT) != 0)
- /* fd denotes an inheritable descriptor. */
- break;
+ DWORD hflags;
+ /* GetHandleInformation
+ <https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-gethandleinformation> */
+ if (GetHandleInformation (handle, &hflags))
+ {
+ if ((hflags & HANDLE_FLAG_INHERIT) != 0)
+ /* fd denotes an inheritable descriptor. */
+ break;
+ }
}
}
}
errno = ENOMEM;
return -1;
}
- unsigned char *flags_array =
- (unsigned char *) malloc (handles_allocated * sizeof (unsigned char));
+ unsigned short *flags_array =
+ (unsigned short *) malloc (handles_allocated * sizeof (unsigned short));
if (flags_array == NULL)
{
free (handles_array);
<https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-gethandleinformation> */
if (GetHandleInformation (handle, &hflags))
{
- if ((hflags & HANDLE_FLAG_INHERIT) != 0)
+ if (duplicate)
{
- /* fd denotes an inheritable descriptor. */
- if (duplicate)
+ /* Add fd to the array, regardless of whether it is
+ inheritable or not. */
+ if (!DuplicateHandle (curr_process, handle,
+ curr_process, &handles_array[fd],
+ 0, TRUE, DUPLICATE_SAME_ACCESS))
{
- if (!DuplicateHandle (curr_process, handle,
- curr_process, &handles_array[fd],
- 0, TRUE, DUPLICATE_SAME_ACCESS))
- {
- unsigned int i;
- for (i = 0; i < fd; i++)
- if (handles_array[i] != INVALID_HANDLE_VALUE)
- CloseHandle (handles_array[i]);
- free (flags_array);
- free (handles_array);
- errno = EBADF; /* arbitrary */
- return -1;
- }
+ unsigned int i;
+ for (i = 0; i < fd; i++)
+ if (handles_array[i] != INVALID_HANDLE_VALUE)
+ CloseHandle (handles_array[i]);
+ free (flags_array);
+ free (handles_array);
+ errno = EBADF; /* arbitrary */
+ return -1;
+ }
+ flags_array[fd] =
+ ((hflags & HANDLE_FLAG_INHERIT) != 0 ? KEEP_OPEN_IN_CHILD : 0);
+ }
+ else
+ {
+ if ((hflags & HANDLE_FLAG_INHERIT) != 0)
+ {
+ /* fd denotes an inheritable descriptor. */
+ handles_array[fd] = handle;
+ flags_array[fd] = KEEP_OPEN_IN_CHILD;
}
- else
- handles_array[fd] = handle;
-
- flags_array[fd] = 0;
}
}
}
if (handle != INVALID_HANDLE_VALUE
/* The first three are possibly already passed above.
But they need to passed here as well, if they have some flags. */
- && (fd >= 3 || inh_handles->flags[fd] != 0))
+ && (fd >= 3 || (unsigned char) inh_handles->flags[fd] != 0))
{
DWORD hflags;
/* GetHandleInformation
flags[fd] = 1. But on ReactOS or Wine, adding the bit
that indicates the handle type may be necessary. So,
just do it everywhere. */
- flags[fd] = 1 | inh_handles->flags[fd];
+ flags[fd] = 1 | (unsigned char) inh_handles->flags[fd];
switch (GetFileType (handle))
{
case FILE_TYPE_CHAR:
errno = saved_errno;
return -1;
}
- inh_handles.handles[0] = stdin_handle; inh_handles.flags[0] = 0;
- inh_handles.handles[1] = stdout_handle; inh_handles.flags[1] = 0;
- inh_handles.handles[2] = stderr_handle; inh_handles.flags[2] = 0;
+ inh_handles.handles[0] = stdin_handle;
+ inh_handles.flags[0] = KEEP_OPEN_IN_CHILD;
+ inh_handles.handles[1] = stdout_handle;
+ inh_handles.flags[1] = KEEP_OPEN_IN_CHILD;
+ inh_handles.handles[2] = stderr_handle;
+ inh_handles.flags[2] = KEEP_OPEN_IN_CHILD;
/* CreateProcess
<https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa> */
_GL_ATTRIBUTE_MALLOC _GL_ATTRIBUTE_DEALLOC_FREE;
-/* This struct keeps track of which handles to pass to a subprocess, and with
- which flags. All of the handles here are inheritable.
+/* This struct keeps track of which handles to potentially pass to a subprocess,
+ and with which flags. All of the handles here are inheritable.
Regarding handle inheritance, see
- <https://docs.microsoft.com/en-us/windows/win32/sysinfo/handle-inheritance> */
+ <https://docs.microsoft.com/en-us/windows/win32/sysinfo/handle-inheritance>.
+ Whether a handle is actually scheduled for being preserved in the child
+ process is determined by the KEEP_OPEN_IN_CHILD bit in the flags. */
struct inheritable_handles
{
/* The number of occupied entries in the two arrays below.
flags[fd] is only relevant if handles[fd] != INVALID_HANDLE_VALUE.
It is a bit mask consisting of:
- 32 for O_APPEND.
+ - KEEP_OPEN_IN_CHILD if handles[fd] is scheduled to be preserved in the
+ child process.
*/
- unsigned char *flags;
+ unsigned short *flags;
+ #define KEEP_OPEN_IN_CHILD 0x100
};
-/* Initializes a set of inheritable handles, filling in all inheritable handles
- assigned to file descriptors.
- If DUPLICATE is true, the handles stored in the set are duplicates.
+/* Initializes a set of inheritable handles, filling in all or part of the
+ file descriptors of the current process.
+ If DUPLICATE is true, the handles stored are those of all file descriptors,
+ and we use DuplicateHandle to make sure that they are all inheritable.
+ If DUPLICATE is false, the handles stored are only the inheritables ones;
+ this is a shortcut for spawnpvech().
Returns 0 upon success. In case of failure, -1 is returned, with errno set.
*/
extern int init_inheritable_handles (struct inheritable_handles *inh_handles,