>From 3b68fa987fef59df34752a117d6df872f1d97b6c Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Sat, 10 Sep 2022 02:27:05 +0200 Subject: [PATCH 2/5] posix_spawn-internal: Optimize DuplicateHandle calls on native Windows. * lib/windows-spawn.h (KEEP_OPEN_IN_PARENT): New macro. * lib/windows-spawn.c (init_inheritable_handles): When a handle is already inheritable, don't bother duplicating it; instead, just mark it as KEEP_OPEN_IN_PARENT. * lib/spawni.c (shrink_inheritable_handles, close_inheritable_handles, do_open, do_dup2, do_close): Don't close handles that are marked as KEEP_OPEN_IN_PARENT. --- ChangeLog | 11 +++++++++++ lib/spawni.c | 12 +++++++++--- lib/windows-spawn.c | 35 ++++++++++++++++++++++------------- lib/windows-spawn.h | 3 +++ 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3b4db6dfff..6ecba7fa81 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2022-09-09 Bruno Haible + + posix_spawn-internal: Optimize DuplicateHandle calls on native Windows. + * lib/windows-spawn.h (KEEP_OPEN_IN_PARENT): New macro. + * lib/windows-spawn.c (init_inheritable_handles): When a handle is + already inheritable, don't bother duplicating it; instead, just mark it + as KEEP_OPEN_IN_PARENT. + * lib/spawni.c (shrink_inheritable_handles, close_inheritable_handles, + do_open, do_dup2, do_close): Don't close handles that are marked as + KEEP_OPEN_IN_PARENT. + 2022-09-09 Bruno Haible posix_spawn-internal: Don't lose flags while duplicating an fd. diff --git a/lib/spawni.c b/lib/spawni.c index b9b0589460..17a004a38b 100644 --- a/lib/spawni.c +++ b/lib/spawni.c @@ -168,7 +168,8 @@ shrink_inheritable_handles (struct inheritable_handles *inh_handles) if (handle != INVALID_HANDLE_VALUE && (flags[fd] & KEEP_OPEN_IN_CHILD) == 0) { - CloseHandle (handle); + if (!(flags[fd] & KEEP_OPEN_IN_PARENT)) + CloseHandle (handle); handles[fd] = INVALID_HANDLE_VALUE; } } @@ -185,6 +186,7 @@ static void close_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; @@ -192,7 +194,8 @@ close_inheritable_handles (struct inheritable_handles *inh_handles) { HANDLE handle = handles[fd]; - if (handle != INVALID_HANDLE_VALUE) + if (handle != INVALID_HANDLE_VALUE + && !(flags[fd] & KEEP_OPEN_IN_PARENT)) CloseHandle (handle); } } @@ -391,6 +394,7 @@ do_open (struct inheritable_handles *inh_handles, int newfd, if (grow_inheritable_handles (inh_handles, newfd) < 0) return -1; if (inh_handles->handles[newfd] != INVALID_HANDLE_VALUE + && !(inh_handles->flags[newfd] & KEEP_OPEN_IN_PARENT) && !CloseHandle (inh_handles->handles[newfd])) { errno = EIO; @@ -457,6 +461,7 @@ do_dup2 (struct inheritable_handles *inh_handles, int oldfd, int newfd, if (grow_inheritable_handles (inh_handles, newfd) < 0) return -1; if (inh_handles->handles[newfd] != INVALID_HANDLE_VALUE + && !(inh_handles->flags[newfd] & KEEP_OPEN_IN_PARENT) && !CloseHandle (inh_handles->handles[newfd])) { errno = EIO; @@ -489,7 +494,8 @@ do_close (struct inheritable_handles *inh_handles, int fd) errno = EBADF; return -1; } - if (!CloseHandle (inh_handles->handles[fd])) + if (!(inh_handles->flags[fd] & KEEP_OPEN_IN_PARENT) + && !CloseHandle (inh_handles->handles[fd])) { errno = EIO; return -1; diff --git a/lib/windows-spawn.c b/lib/windows-spawn.c index 4c55be0c34..d38d1dc911 100644 --- a/lib/windows-spawn.c +++ b/lib/windows-spawn.c @@ -385,21 +385,30 @@ init_inheritable_handles (struct inheritable_handles *inh_handles, { /* 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 ((hflags & HANDLE_FLAG_INHERIT) != 0) + { + /* Instead of duplicating it, just mark it as shared. */ + handles_array[fd] = handle; + flags_array[fd] = KEEP_OPEN_IN_PARENT | KEEP_OPEN_IN_CHILD; + } + else { - 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; + 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 + && !(flags_array[i] & KEEP_OPEN_IN_PARENT)) + CloseHandle (handles_array[i]); + free (flags_array); + free (handles_array); + errno = EBADF; /* arbitrary */ + return -1; + } + flags_array[fd] = 0; } - flags_array[fd] = - ((hflags & HANDLE_FLAG_INHERIT) != 0 ? KEEP_OPEN_IN_CHILD : 0); } else { diff --git a/lib/windows-spawn.h b/lib/windows-spawn.h index 4414e1b3c7..fdb45a4f15 100644 --- a/lib/windows-spawn.h +++ b/lib/windows-spawn.h @@ -107,9 +107,12 @@ struct inheritable_handles - 32 for O_APPEND. - KEEP_OPEN_IN_CHILD if handles[fd] is scheduled to be preserved in the child process. + - KEEP_OPEN_IN_PARENT if handles[fd] is shared with (and needs to be kept + open in) the parent process. */ unsigned short *flags; #define KEEP_OPEN_IN_CHILD 0x100 + #define KEEP_OPEN_IN_PARENT 0x200 }; /* Initializes a set of inheritable handles, filling in all or part of the -- 2.34.1