luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-08-23 17:38:54 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'revert-it-all-and-let-god-sort-em-out' into 'main'

Browse Source 
Revert "Handling collision between standard i/o file descriptors and newly created ones"

Closes #2795, #2785, and #16

See merge request GNOME/glib!3029
This commit is contained in:
Simon McVittie
2022-11-02 14:34:39 +00:00
parent 451ed353f5 aaae515e13
commit 92730c6226
5 changed files with 353 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
glib/glib-unix.c
View File

@@ -108,17 +108,6 @@ g_unix_open_pipe (int *fds,
ecode = pipe2 (fds, pipe2_flags); ecode = pipe2 (fds, pipe2_flags);
if (ecode == -1 && errno != ENOSYS) if (ecode == -1 && errno != ENOSYS)
return g_unix_set_error_from_errno (error, errno); return g_unix_set_error_from_errno (error, errno);
/* Don't reassign pipes to stdin, stdout, stderr if closed meanwhile */
else if (fds[0] < 3 || fds[1] < 3)
{
int old_fds[2] = { fds[0], fds[1] };
gboolean result = g_unix_open_pipe (fds, flags, error);
close (old_fds[0]);
close (old_fds[1]);
if (!result)
g_unix_set_error_from_errno (error, errno);
}
else if (ecode == 0) else if (ecode == 0)
return TRUE; return TRUE;
/* Fall through on -ENOSYS, we must be running on an old kernel */ /* Fall through on -ENOSYS, we must be running on an old kernel */
@@ -127,19 +116,6 @@ g_unix_open_pipe (int *fds,
ecode = pipe (fds); ecode = pipe (fds);
if (ecode == -1) if (ecode == -1)
return g_unix_set_error_from_errno (error, errno); return g_unix_set_error_from_errno (error, errno);
/* Don't reassign pipes to stdin, stdout, stderr if closed meanwhile */
else if (fds[0] < 3 || fds[1] < 3)
{
int old_fds[2] = { fds[0], fds[1] };
gboolean result = g_unix_open_pipe (fds, flags, error);
close (old_fds[0]);
close (old_fds[1]);
if (!result)
g_unix_set_error_from_errno (error, errno);
return result;
}
if (flags == 0) if (flags == 0)
return TRUE; return TRUE;

237
glib/gspawn.c
View File

@@ -32,6 +32,7 @@
#include <string.h> #include <string.h>
#include <stdlib.h> /* for fdwalk */ #include <stdlib.h> /* for fdwalk */
#include <dirent.h> #include <dirent.h>
#include <unistd.h>
#ifdef HAVE_SPAWN_H #ifdef HAVE_SPAWN_H
#include <spawn.h> #include <spawn.h>
@@ -73,6 +74,9 @@
#define INHERITS_OR_NULL_STDOUT (G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_CHILD_INHERITS_STDOUT) #define INHERITS_OR_NULL_STDOUT (G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_CHILD_INHERITS_STDOUT)
#define INHERITS_OR_NULL_STDERR (G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_CHILD_INHERITS_STDERR) #define INHERITS_OR_NULL_STDERR (G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_CHILD_INHERITS_STDERR)
#define IS_STD_FILENO(_fd) ((_fd >= STDIN_FILENO) && (_fd <= STDERR_FILENO))
#define IS_VALID_FILENO(_fd) (_fd >= 0)
/* posix_spawn() is assumed the fastest way to spawn, but glibc's /* posix_spawn() is assumed the fastest way to spawn, but glibc's
* implementation was buggy before glibc 2.24, so avoid it on old versions. * implementation was buggy before glibc 2.24, so avoid it on old versions.
*/ */
@@ -1334,8 +1338,12 @@ dupfd_cloexec (int old_fd, int new_fd_min)
return fd; return fd;
} }
/* This function is called between fork() and exec() and hence must be /* fdwalk()-compatible callback to close a valid fd.
* async-signal-safe (see signal-safety(7)). */ * It is an error to pass an invalid fd (causing EBADF) to this function.
*
* This function is called between fork() and exec() and hence must be
* async-signal-safe (see signal-safety(7)).
*/
G_GNUC_UNUSED static int G_GNUC_UNUSED static int
close_func (void *data, int fd) close_func (void *data, int fd)
{ {
@@ -1345,6 +1353,35 @@ close_func (void *data, int fd)
return 0; return 0;
} }
/* fdwalk()-compatible callback to close a fd for non-compliant
* implementations of fdwalk() that potentially pass already
* closed fds.
*
* It is not an error to pass an invalid fd to this function.
*
* This function is called between fork() and exec() and hence must be
* async-signal-safe (see signal-safety(7)).
*/
G_GNUC_UNUSED static int
close_func_with_invalid_fds (void *data, int fd)
{
/* We use close and not g_close here because on some platforms, we
* don't know how to close only valid, open file descriptors, so we
* have to pass bad fds to close too. g_close warns if given a bad
* fd.
*
* This function returns no error, because there is nothing that the caller
* could do with that information. That is even the case for EINTR. See
* g_close() about the specialty of EINTR and why that is correct.
* If g_close() ever gets extended to handle EINTR specially, then this place
* should get updated to do the same handling.
*/
if (fd >= GPOINTER_TO_INT (data))
close (fd);
return 0;
}
#ifdef __linux__ #ifdef __linux__
struct linux_dirent64 struct linux_dirent64
{ {
@@ -1400,18 +1437,14 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
return fdwalk (cb, data); return fdwalk (cb, data);
#else #else
/* Fallback implementation of fdwalk. It should be async-signal safe, but it /* Fallback implementation of fdwalk. It should be async-signal safe, but it
* may be slow on non-Linux operating systems, especially on systems allowing * may fail on non-Linux operating systems. See safe_fdwalk_with_invalid_fds
* very high number of open file descriptors. * for a slower alternative.
*/ */
gint open_max = -1;
gint fd;
gint res = 0;
#if 0 && defined(HAVE_SYS_RESOURCE_H)
struct rlimit rl;
#endif
#ifdef __linux__ #ifdef __linux__
gint fd;
gint res = 0;
/* Avoid use of opendir/closedir since these are not async-signal-safe. */ /* Avoid use of opendir/closedir since these are not async-signal-safe. */
int dir_fd = open ("/proc/self/fd", O_RDONLY | O_DIRECTORY); int dir_fd = open ("/proc/self/fd", O_RDONLY | O_DIRECTORY);
if (dir_fd >= 0) if (dir_fd >= 0)
@@ -1439,7 +1472,8 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
return res; return res;
} }
/* If /proc is not mounted or not accessible we fall back to the old /* If /proc is not mounted or not accessible we fail here and rely on
* safe_fdwalk_with_invalid_fds to fall back to the old
* rlimit trick. */ * rlimit trick. */
#endif #endif
@@ -1455,6 +1489,8 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
* fcntl(fd, F_PREVFD) * fcntl(fd, F_PREVFD)
* - return highest allocated file descriptor < fd. * - return highest allocated file descriptor < fd.
*/ */
gint fd;
gint res = 0;
open_max = fcntl (INT_MAX, F_PREVFD); /* find the maximum fd */ open_max = fcntl (INT_MAX, F_PREVFD); /* find the maximum fd */
if (open_max < 0) /* No open files */ if (open_max < 0) /* No open files */
@@ -1463,9 +1499,31 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
for (fd = -1; (fd = fcntl (fd, F_NEXTFD, open_max)) != -1; ) for (fd = -1; (fd = fcntl (fd, F_NEXTFD, open_max)) != -1; )
if ((res = cb (data, fd)) != 0 || fd == open_max) if ((res = cb (data, fd)) != 0 || fd == open_max)
break; break;
#else
return res;
#endif
errno = ENOSYS;
return -1;
#endif
}
/* This function is called between fork() and exec() and hence must be
* async-signal-safe (see signal-safety(7)). */
static int
safe_fdwalk_with_invalid_fds (int (*cb)(void *data, int fd), void *data)
{
/* Fallback implementation of fdwalk. It should be async-signal safe, but it
* may be slow, especially on systems allowing very high number of open file
* descriptors.
*/
gint open_max = -1;
gint fd;
gint res = 0;
#if 0 && defined(HAVE_SYS_RESOURCE_H) #if 0 && defined(HAVE_SYS_RESOURCE_H)
struct rlimit rl;
/* Use getrlimit() function provided by the system if it is known to be /* Use getrlimit() function provided by the system if it is known to be
* async-signal safe. * async-signal safe.
* *
@@ -1498,10 +1556,8 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
for (fd = 0; fd < open_max; fd++) for (fd = 0; fd < open_max; fd++)
if ((res = cb (data, fd)) != 0) if ((res = cb (data, fd)) != 0)
break; break;
#endif
return res; return res;
#endif
} }
/* This function is called between fork() and exec() and hence must be /* This function is called between fork() and exec() and hence must be
@@ -1509,6 +1565,8 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
static int static int
safe_fdwalk_set_cloexec (int lowfd) safe_fdwalk_set_cloexec (int lowfd)
{ {
int ret;
#if defined(HAVE_CLOSE_RANGE) && defined(CLOSE_RANGE_CLOEXEC) #if defined(HAVE_CLOSE_RANGE) && defined(CLOSE_RANGE_CLOEXEC)
/* close_range() is available in Linux since kernel 5.9, and on FreeBSD at /* close_range() is available in Linux since kernel 5.9, and on FreeBSD at
* around the same time. It was designed for use in async-signal-safe * around the same time. It was designed for use in async-signal-safe
@@ -1520,11 +1578,17 @@ safe_fdwalk_set_cloexec (int lowfd)
* Handle ENOSYS in case it’s supported in libc but not the kernel; if so, * Handle ENOSYS in case it’s supported in libc but not the kernel; if so,
* fall back to safe_fdwalk(). Handle EINVAL in case `CLOSE_RANGE_CLOEXEC` * fall back to safe_fdwalk(). Handle EINVAL in case `CLOSE_RANGE_CLOEXEC`
* is not supported. */ * is not supported. */
int ret = close_range (lowfd, G_MAXUINT, CLOSE_RANGE_CLOEXEC); ret = close_range (lowfd, G_MAXUINT, CLOSE_RANGE_CLOEXEC);
if (ret == 0 || !(errno == ENOSYS || errno == EINVAL)) if (ret == 0 || !(errno == ENOSYS || errno == EINVAL))
return ret; return ret;
#endif /* HAVE_CLOSE_RANGE */ #endif /* HAVE_CLOSE_RANGE */
return safe_fdwalk (set_cloexec, GINT_TO_POINTER (lowfd));
ret = safe_fdwalk (set_cloexec, GINT_TO_POINTER (lowfd));
if (ret < 0 && errno == ENOSYS)
ret = safe_fdwalk_with_invalid_fds (set_cloexec, GINT_TO_POINTER (lowfd));
return ret;
} }
/* This function is called between fork() and exec() and hence must be /* This function is called between fork() and exec() and hence must be
@@ -1534,6 +1598,8 @@ safe_fdwalk_set_cloexec (int lowfd)
static int static int
safe_closefrom (int lowfd) safe_closefrom (int lowfd)
{ {
int ret;
#if defined(__FreeBSD__) || defined(__OpenBSD__) || \ #if defined(__FreeBSD__) || defined(__OpenBSD__) || \
(defined(__sun__) && defined(F_CLOSEFROM)) (defined(__sun__) && defined(F_CLOSEFROM))
/* Use closefrom function provided by the system if it is known to be /* Use closefrom function provided by the system if it is known to be
@@ -1573,11 +1639,16 @@ safe_closefrom (int lowfd)
* *
* Handle ENOSYS in case it’s supported in libc but not the kernel; if so, * Handle ENOSYS in case it’s supported in libc but not the kernel; if so,
* fall back to safe_fdwalk(). */ * fall back to safe_fdwalk(). */
int ret = close_range (lowfd, G_MAXUINT, 0); ret = close_range (lowfd, G_MAXUINT, 0);
if (ret == 0 || errno != ENOSYS) if (ret == 0 || errno != ENOSYS)
return ret; return ret;
#endif /* HAVE_CLOSE_RANGE */ #endif /* HAVE_CLOSE_RANGE */
return safe_fdwalk (close_func, GINT_TO_POINTER (lowfd)); ret = safe_fdwalk (close_func, GINT_TO_POINTER (lowfd));
if (ret < 0 && errno == ENOSYS)
ret = safe_fdwalk_with_invalid_fds (close_func_with_invalid_fds, GINT_TO_POINTER (lowfd));
return ret;
#endif #endif
} }
@@ -1595,6 +1666,30 @@ safe_dup2 (gint fd1, gint fd2)
return ret; return ret;
} }
/* This function is called between fork() and exec() and hence must be
* async-signal-safe (see signal-safety(7)). */
static gboolean
relocate_fd_out_of_standard_range (gint *fd)
{
gint ret = -1;
const int min_fileno = STDERR_FILENO + 1;
do
ret = fcntl (*fd, F_DUPFD, min_fileno);
while (ret < 0 && errno == EINTR);
/* Note we don't need to close the old fd, because the caller is expected
* to close fds in the standard range itself.
*/
if (ret >= min_fileno)
{
*fd = ret;
return TRUE;
}
return FALSE;
}
/* This function is called between fork() and exec() and hence must be /* This function is called between fork() and exec() and hence must be
* async-signal-safe (see signal-safety(7)). */ * async-signal-safe (see signal-safety(7)). */
static gint static gint
@@ -1614,7 +1709,7 @@ enum
CHILD_CHDIR_FAILED, CHILD_CHDIR_FAILED,
CHILD_EXEC_FAILED, CHILD_EXEC_FAILED,
CHILD_OPEN_FAILED, CHILD_OPEN_FAILED,
CHILD_DUP2_FAILED, CHILD_DUPFD_FAILED,
CHILD_FORK_FAILED, CHILD_FORK_FAILED,
CHILD_CLOSE_FAILED, CHILD_CLOSE_FAILED,
}; };
@@ -1653,17 +1748,42 @@ do_exec (gint child_err_report_fd,
if (working_directory && chdir (working_directory) < 0) if (working_directory && chdir (working_directory) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_CHDIR_FAILED); CHILD_CHDIR_FAILED);
/* Redirect pipes as required */ /* It's possible the caller assigned stdin to an fd with a
if (stdin_fd >= 0) * file number that is supposed to be reserved for
* stdout or stderr.
*
* If so, move it up out of the standard range, so it doesn't
* cause a conflict.
*/
if (IS_STD_FILENO (stdin_fd) && stdin_fd != STDIN_FILENO)
{
int old_fd = stdin_fd;
if (!relocate_fd_out_of_standard_range (&stdin_fd))
write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
if (stdout_fd == old_fd)
stdout_fd = stdin_fd;
if (stderr_fd == old_fd)
stderr_fd = stdin_fd;
}
/* Redirect pipes as required
*
* There are two cases where we don't need to do the redirection
* 1. Where the associated file descriptor is cleared/invalid
* 2. When the associated file descriptor is already given the
* correct file number.
*/
if (IS_VALID_FILENO (stdin_fd) && stdin_fd != STDIN_FILENO)
{ {
if (safe_dup2 (stdin_fd, 0) < 0) if (safe_dup2 (stdin_fd, 0) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUPFD_FAILED);
if (!((stdout_fd >= 0 || stdout_to_null) && stdin_fd == 1) && set_cloexec (GINT_TO_POINTER(0), stdin_fd);
!((stderr_fd >= 0 || stderr_to_null) && stdin_fd == 2))
set_cloexec (GINT_TO_POINTER(0), stdin_fd);
} }
else if (!child_inherits_stdin) else if (!child_inherits_stdin)
{ {
@@ -1674,19 +1794,34 @@ do_exec (gint child_err_report_fd,
CHILD_OPEN_FAILED); CHILD_OPEN_FAILED);
if (safe_dup2 (read_null, 0) < 0) if (safe_dup2 (read_null, 0) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUPFD_FAILED);
close_and_invalidate (&read_null); close_and_invalidate (&read_null);
} }
if (stdout_fd >= 0) /* Like with stdin above, it's possible the caller assigned
* stdout to an fd with a file number that's intruding on the
* standard range.
*
* If so, move it out of the way, too.
*/
if (IS_STD_FILENO (stdout_fd) && stdout_fd != STDOUT_FILENO)
{
int old_fd = stdout_fd;
if (!relocate_fd_out_of_standard_range (&stdout_fd))
write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
if (stderr_fd == old_fd)
stderr_fd = stdout_fd;
}
if (IS_VALID_FILENO (stdout_fd) && stdout_fd != STDOUT_FILENO)
{ {
if (safe_dup2 (stdout_fd, 1) < 0) if (safe_dup2 (stdout_fd, 1) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUPFD_FAILED);
if (!((stdin_fd >= 0 || !child_inherits_stdin) && stdout_fd == 0) && set_cloexec (GINT_TO_POINTER(0), stdout_fd);
!((stderr_fd >= 0 || stderr_to_null) && stdout_fd == 2))
set_cloexec (GINT_TO_POINTER(0), stdout_fd);
} }
else if (stdout_to_null) else if (stdout_to_null)
{ {
@@ -1696,19 +1831,29 @@ do_exec (gint child_err_report_fd,
CHILD_OPEN_FAILED); CHILD_OPEN_FAILED);
if (safe_dup2 (write_null, 1) < 0) if (safe_dup2 (write_null, 1) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUPFD_FAILED);
close_and_invalidate (&write_null); close_and_invalidate (&write_null);
} }
if (stderr_fd >= 0) if (IS_STD_FILENO (stderr_fd) && stderr_fd != STDERR_FILENO)
{
if (!relocate_fd_out_of_standard_range (&stderr_fd))
write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
}
/* Like with stdin/stdout above, it's possible the caller assigned
* stderr to an fd with a file number that's intruding on the
* standard range.
*
* Make sure it's out of the way, also.
*/
if (IS_VALID_FILENO (stderr_fd) && stderr_fd != STDERR_FILENO)
{ {
if (safe_dup2 (stderr_fd, 2) < 0) if (safe_dup2 (stderr_fd, 2) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUPFD_FAILED);
if (!((stdin_fd >= 0 || !child_inherits_stdin) && stderr_fd == 0) && set_cloexec (GINT_TO_POINTER(0), stderr_fd);
!((stdout_fd >= 0 || stdout_to_null) && stderr_fd == 1))
set_cloexec (GINT_TO_POINTER(0), stderr_fd);
} }
else if (stderr_to_null) else if (stderr_to_null)
{ {
@@ -1718,7 +1863,7 @@ do_exec (gint child_err_report_fd,
CHILD_OPEN_FAILED); CHILD_OPEN_FAILED);
if (safe_dup2 (write_null, 2) < 0) if (safe_dup2 (write_null, 2) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUPFD_FAILED);
close_and_invalidate (&write_null); close_and_invalidate (&write_null);
} }
@@ -1732,7 +1877,7 @@ do_exec (gint child_err_report_fd,
if (child_setup == NULL && n_fds == 0) if (child_setup == NULL && n_fds == 0)
{ {
if (safe_dup2 (child_err_report_fd, 3) < 0) if (safe_dup2 (child_err_report_fd, 3) < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED); write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
set_cloexec (GINT_TO_POINTER (0), 3); set_cloexec (GINT_TO_POINTER (0), 3);
if (safe_closefrom (4) < 0) if (safe_closefrom (4) < 0)
write_err_and_exit (child_err_report_fd, CHILD_CLOSE_FAILED); write_err_and_exit (child_err_report_fd, CHILD_CLOSE_FAILED);
@@ -1768,7 +1913,7 @@ do_exec (gint child_err_report_fd,
if (max_target_fd == G_MAXINT) if (max_target_fd == G_MAXINT)
{ {
errno = EINVAL; errno = EINVAL;
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED); write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
} }
/* If we're doing remapping fd assignments, we need to handle /* If we're doing remapping fd assignments, we need to handle
@@ -1782,7 +1927,7 @@ do_exec (gint child_err_report_fd,
{ {
source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1); source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1);
if (source_fds[i] < 0) if (source_fds[i] < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED); write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
} }
} }
@@ -1804,11 +1949,11 @@ do_exec (gint child_err_report_fd,
{ {
child_err_report_fd = dupfd_cloexec (child_err_report_fd, max_target_fd + 1); child_err_report_fd = dupfd_cloexec (child_err_report_fd, max_target_fd + 1);
if (child_err_report_fd < 0) if (child_err_report_fd < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED); write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
} }
if (safe_dup2 (source_fds[i], target_fds[i]) < 0) if (safe_dup2 (source_fds[i], target_fds[i]) < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED); write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
close_and_invalidate (&source_fds[i]); close_and_invalidate (&source_fds[i]);
} }
@@ -2528,7 +2673,7 @@ fork_exec (gboolean intermediate_child,
g_strerror (buf[1])); g_strerror (buf[1]));
break; break;
case CHILD_DUP2_FAILED: case CHILD_DUPFD_FAILED:
g_set_error (error, g_set_error (error,
G_SPAWN_ERROR, G_SPAWN_ERROR,
G_SPAWN_ERROR_FAILED, G_SPAWN_ERROR_FAILED,

8
glib/gstdio.c
View File

@@ -1788,6 +1788,8 @@ g_close (gint fd,
* on Linux at least. Anyone who wants to add a conditional check * on Linux at least. Anyone who wants to add a conditional check
* for e.g. HP-UX is welcome to do so later... * for e.g. HP-UX is welcome to do so later...
* *
* close_func_with_invalid_fds() in gspawn.c has similar logic.
*
* https://lwn.net/Articles/576478/ * https://lwn.net/Articles/576478/
* http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html * http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
* https://bugzilla.gnome.org/show_bug.cgi?id=682819 * https://bugzilla.gnome.org/show_bug.cgi?id=682819
@@ -1814,13 +1816,7 @@ g_close (gint fd,
* not necessarily in the caller of g_close(), but somebody else * not necessarily in the caller of g_close(), but somebody else
* might have wrongly closed fd. In any case, there is a serious bug * might have wrongly closed fd. In any case, there is a serious bug
* somewhere. */ * somewhere. */
/* FIXME: This causes a number of unit test failures on macOS.
* Disabling the message for now until someone with access to a
* macOS machine can investigate.
* See https://gitlab.gnome.org/GNOME/glib/-/issues/2785 */
#ifndef G_OS_DARWIN
g_critical ("g_close(fd:%d) failed with EBADF. The tracking of file descriptors got messed up", fd); g_critical ("g_close(fd:%d) failed with EBADF. The tracking of file descriptors got messed up", fd);
#endif
} }
else else
{ {

119
glib/tests/spawn-test.c
View File

@@ -27,6 +27,10 @@
#include <glib.h> #include <glib.h>
#include <glib/gstdio.h> #include <glib/gstdio.h>
#ifdef G_OS_UNIX
#include <unistd.h>
#endif
#ifdef G_OS_WIN32 #ifdef G_OS_WIN32
#include <fcntl.h> #include <fcntl.h>
#include <io.h> #include <io.h>
@@ -208,6 +212,118 @@ test_spawn_basics (void)
#endif #endif
} }
#ifdef G_OS_UNIX
static void
test_spawn_stdio_overwrite (void)
{
gboolean result;
int ret;
GError *error = NULL;
int old_stdin_fd = -1;
int old_stdout_fd = -1;
int old_stderr_fd = -1;
char **envp = g_get_environ ();
enum OpenState { OPENED = 0, CLOSED = 1, DONE = 2 } stdin_state, stdout_state, stderr_state, output_return_state, error_return_state;
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/16");
old_stdin_fd = dup (STDIN_FILENO);
old_stdout_fd = dup (STDOUT_FILENO);
old_stderr_fd = dup (STDERR_FILENO);
for (output_return_state = OPENED; output_return_state != DONE; output_return_state++)
for (error_return_state = OPENED; error_return_state != DONE; error_return_state++)
for (stdin_state = OPENED; stdin_state != DONE; stdin_state++)
for (stdout_state = OPENED; stdout_state != DONE; stdout_state++)
for (stderr_state = OPENED; stderr_state != DONE; stderr_state++)
{
char *command_line = NULL;
char **argv = NULL;
gchar *standard_output = NULL;
gchar *standard_error = NULL;
g_test_message ("Fetching GSpawn result %s%s%s with stdin %s, stdout %s, stderr %s",
output_return_state == OPENED? "output" : "",
output_return_state == OPENED && error_return_state == OPENED? " and " : "",
error_return_state == OPENED? "error output" : "",
stdin_state == CLOSED? "already closed" : "open",
stdout_state == CLOSED? "already closed" : "open",
stderr_state == CLOSED? "already closed" : "open");
if (stdin_state == CLOSED)
{
g_close (STDIN_FILENO, &error);
g_assert_no_error (error);
}
if (stdout_state == CLOSED)
{
g_close (STDOUT_FILENO, &error);
g_assert_no_error (error);
}
if (stderr_state == CLOSED)
{
g_close (STDERR_FILENO, &error);
g_assert_no_error (error);
}
command_line = g_strdup_printf ("/bin/sh -c '%s%s%s'",
output_return_state == OPENED? "echo stdout": "",
output_return_state == OPENED && error_return_state == OPENED? ";" : "",
error_return_state == OPENED? "echo stderr >&2": "");
g_shell_parse_argv (command_line, NULL, &argv, &error);
g_assert_no_error (error);
g_clear_pointer (&command_line, g_free);
result = g_spawn_sync (NULL,
argv, envp, G_SPAWN_SEARCH_PATH_FROM_ENVP,
NULL, NULL,
output_return_state == OPENED? &standard_output : NULL,
error_return_state == OPENED? &standard_error: NULL,
NULL,
&error);
g_clear_pointer (&argv, g_strfreev);
ret = dup2 (old_stderr_fd, STDERR_FILENO);
g_assert_cmpint (ret, ==, STDERR_FILENO);
ret = dup2 (old_stdout_fd, STDOUT_FILENO);
g_assert_cmpint (ret, ==, STDOUT_FILENO);
ret = dup2 (old_stdin_fd, STDIN_FILENO);
g_assert_cmpint (ret, ==, STDIN_FILENO);
g_assert_no_error (error);
g_assert_true (result);
if (output_return_state == OPENED)
{
g_assert_cmpstr (standard_output, ==, "stdout\n");
g_clear_pointer (&standard_output, g_free);
}
if (error_return_state == OPENED)
{
g_assert_cmpstr (standard_error, ==, "stderr\n");
g_clear_pointer (&standard_error, g_free);
}
}
g_clear_fd (&old_stdin_fd, &error);
g_assert_no_error (error);
g_clear_fd (&old_stdout_fd, &error);
g_assert_no_error (error);
g_clear_fd (&old_stderr_fd, &error);
g_assert_no_error (error);
g_clear_pointer (&envp, g_strfreev);
}
#endif
int int
main (int argc, main (int argc,
char *argv[]) char *argv[])
@@ -219,6 +335,9 @@ main (int argc,
g_test_init (&argc, &argv, NULL); g_test_init (&argc, &argv, NULL);
g_test_add_func ("/spawn/basics", test_spawn_basics); g_test_add_func ("/spawn/basics", test_spawn_basics);
#ifdef G_OS_UNIX
g_test_add_func ("/spawn/stdio-overwrite", test_spawn_stdio_overwrite);
#endif
return g_test_run (); return g_test_run ();
} }

41
glib/tests/unix.c
View File

@@ -26,8 +26,11 @@
#include "config.h" #include "config.h"
#include "glib-unix.h" #include "glib-unix.h"
#include "gstdio.h"
#include <string.h> #include <string.h>
#include <pwd.h> #include <pwd.h>
#include <unistd.h>
static void static void
test_pipe (void) test_pipe (void)
@@ -54,6 +57,43 @@ test_pipe (void)
g_assert (g_str_has_prefix (buf, "hello")); g_assert (g_str_has_prefix (buf, "hello"));
} }
static void
test_pipe_stdio_overwrite (void)
{
GError *error = NULL;
int pipefd[2], ret;
gboolean res;
int stdin_fd;
g_test_summary ("Test that g_unix_open_pipe() will use the first available FD, even if it’s stdin/stdout/stderr");
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2795");
stdin_fd = dup (STDIN_FILENO);
g_assert_cmpint (stdin_fd, >, 0);
g_close (STDIN_FILENO, &error);
g_assert_no_error (error);
res = g_unix_open_pipe (pipefd, FD_CLOEXEC, &error);
g_assert_no_error (error);
g_assert_true (res);
g_assert_cmpint (pipefd[0], ==, STDIN_FILENO);
g_close (pipefd[0], &error);
g_assert_no_error (error);
g_close (pipefd[1], &error);
g_assert_no_error (error);
ret = dup2 (stdin_fd, STDIN_FILENO);
g_assert_cmpint (ret, >=, 0);
g_close (stdin_fd, &error);
g_assert_no_error (error);
}
static void static void
test_error (void) test_error (void)
{ {
@@ -339,6 +379,7 @@ main (int argc,
g_test_init (&argc, &argv, NULL); g_test_init (&argc, &argv, NULL);
g_test_add_func ("/glib-unix/pipe", test_pipe); g_test_add_func ("/glib-unix/pipe", test_pipe);
g_test_add_func ("/glib-unix/pipe-stdio-overwrite", test_pipe_stdio_overwrite);
g_test_add_func ("/glib-unix/error", test_error); g_test_add_func ("/glib-unix/error", test_error);
g_test_add_func ("/glib-unix/nonblocking", test_nonblocking); g_test_add_func ("/glib-unix/nonblocking", test_nonblocking);
g_test_add_func ("/glib-unix/sighup", test_sighup); g_test_add_func ("/glib-unix/sighup", test_sighup);