diff --git a/SOURCES/2408.patch b/SOURCES/2408.patch new file mode 100644 index 0000000..90d8953 --- /dev/null +++ b/SOURCES/2408.patch @@ -0,0 +1,391 @@ +From 0bbd63bf1945c6f3e1c88232521e1618c21d44f2 Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Thu, 23 Dec 2021 17:45:51 +0000 +Subject: [PATCH 1/4] gmain: Use waitid() on pidfds rather than a global + SIGCHLD handler +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When the system supports it (as all Linux kernels ≥ 5.3 should), it’s +preferable to use `pidfd_open()` and `waitid()` to be notified of +child processes exiting or being signalled, rather than installing a +default `SIGCHLD` handler. + +A default `SIGCHLD` handler is global, and can never interact well with +other code (from the application or other libraries) which also wants to +install a `SIGCHLD` handler. + +This use of `pidfd_open()` is racy (the PID may be reused between +`g_child_watch_source_new()` being called and `pidfd_open()` being +called), so it doesn’t improve behaviour there. For that, we’d need +continuous use of pidfds throughout GLib, from fork/spawn time until +here. See #1866 for that. + +The use of `waitid()` to get the process exit status could be expanded +in future to also work for stopped or continued processes (as per #175) +by adding `WSTOPPED | WCONTINUED` into the flags. That’s a behaviour +change which is outside the strict scope of adding pidfd support, +though. + +Signed-off-by: Philip Withnall + +Helps: #1866 +Fixes: #2216 +--- + glib/gmain.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++--- + meson.build | 14 ++++++ + 2 files changed, 125 insertions(+), 6 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index 15581ee7a..e9965f7f6 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -67,6 +67,12 @@ + #include + #include + ++#ifdef HAVE_PIDFD ++#include ++#include ++#include /* P_PIDFD */ ++#endif /* HAVE_PIDFD */ ++ + #ifdef G_OS_WIN32 + #define STRICT + #include +@@ -329,10 +335,11 @@ struct _GChildWatchSource + GSource source; + GPid pid; + gint child_status; +-#ifdef G_OS_WIN32 ++ /* @poll is always used on Windows, and used on Unix iff @using_pidfd is set: */ + GPollFD poll; +-#else /* G_OS_WIN32 */ +- gboolean child_exited; /* (atomic) */ ++#ifndef G_OS_WIN32 ++ gboolean child_exited; /* (atomic); not used iff @using_pidfd is set */ ++ gboolean using_pidfd; + #endif /* G_OS_WIN32 */ + }; + +@@ -5325,7 +5332,8 @@ dispatch_unix_signals_unlocked (void) + { + GChildWatchSource *source = node->data; + +- if (!g_atomic_int_get (&source->child_exited)) ++ if (!source->using_pidfd && ++ !g_atomic_int_get (&source->child_exited)) + { + pid_t pid; + do +@@ -5384,6 +5392,38 @@ g_child_watch_prepare (GSource *source, + return g_atomic_int_get (&child_watch_source->child_exited); + } + ++#ifdef HAVE_PIDFD ++static int ++siginfo_t_to_wait_status (const siginfo_t *info) ++{ ++ /* Each of these returns is essentially the inverse of WIFEXITED(), ++ * WIFSIGNALED(), etc. */ ++ switch (info->si_code) ++ { ++ case CLD_EXITED: ++ return W_EXITCODE (info->si_status, 0); ++ case CLD_KILLED: ++ return W_EXITCODE (0, info->si_status); ++ case CLD_DUMPED: ++#ifdef WCOREFLAG ++ return W_EXITCODE (0, info->si_status | WCOREFLAG); ++#else ++ g_assert_not_reached (); ++#endif ++ case CLD_CONTINUED: ++#ifdef __W_CONTINUED ++ return __W_CONTINUED; ++#else ++ g_assert_not_reached (); ++#endif ++ case CLD_STOPPED: ++ case CLD_TRAPPED: ++ default: ++ return W_STOPCODE (info->si_status); ++ } ++} ++#endif /* HAVE_PIDFD */ ++ + static gboolean + g_child_watch_check (GSource *source) + { +@@ -5391,6 +5431,34 @@ g_child_watch_check (GSource *source) + + child_watch_source = (GChildWatchSource *) source; + ++#ifdef HAVE_PIDFD ++ if (child_watch_source->using_pidfd) ++ { ++ gboolean child_exited = child_watch_source->poll.revents & G_IO_IN; ++ ++ if (child_exited) ++ { ++ siginfo_t child_info = { 0, }; ++ ++ /* Get the exit status */ ++ if (waitid (P_PIDFD, child_watch_source->poll.fd, &child_info, WEXITED | WNOHANG) >= 0 && ++ child_info.si_pid != 0) ++ { ++ /* waitid() helpfully provides the wait status in a decomposed ++ * form which is quite useful. Unfortunately we have to report it ++ * to the #GChildWatchFunc as a waitpid()-style platform-specific ++ * wait status, so that the user code in #GChildWatchFunc can then ++ * call WIFEXITED() (etc.) on it. That means re-composing the ++ * status information. */ ++ child_watch_source->child_status = siginfo_t_to_wait_status (&child_info); ++ child_watch_source->child_exited = TRUE; ++ } ++ } ++ ++ return child_exited; ++ } ++#endif /* HAVE_PIDFD */ ++ + return g_atomic_int_get (&child_watch_source->child_exited); + } + +@@ -5575,6 +5643,11 @@ g_unix_signal_watch_finalize (GSource *source) + static void + g_child_watch_finalize (GSource *source) + { ++ GChildWatchSource *child_watch_source = (GChildWatchSource *) source; ++ ++ if (child_watch_source->using_pidfd) ++ return; ++ + G_LOCK (unix_signal_lock); + unix_child_watches = g_slist_remove (unix_child_watches, source); + unref_unix_signal_handler_unlocked (SIGCHLD); +@@ -5676,6 +5749,9 @@ g_child_watch_source_new (GPid pid) + { + GSource *source; + GChildWatchSource *child_watch_source; ++#ifdef HAVE_PIDFD ++ int errsv; ++#endif + + #ifndef G_OS_WIN32 + g_return_val_if_fail (pid > 0, NULL); +@@ -5694,14 +5770,43 @@ g_child_watch_source_new (GPid pid) + child_watch_source->poll.events = G_IO_IN; + + g_source_add_poll (source, &child_watch_source->poll); +-#else /* G_OS_WIN32 */ ++#else /* !G_OS_WIN32 */ ++ ++#ifdef HAVE_PIDFD ++ /* Use a pidfd, if possible, to avoid having to install a global SIGCHLD ++ * handler and potentially competing with any other library/code which wants ++ * to install one. ++ * ++ * Unfortunately this use of pidfd isn’t race-free (the PID could be recycled ++ * between the caller calling g_child_watch_source_new() and here), but it’s ++ * better than SIGCHLD. ++ */ ++ child_watch_source->poll.fd = (int) syscall (SYS_pidfd_open, pid, 0); ++ errsv = errno; ++ ++ if (child_watch_source->poll.fd >= 0) ++ { ++ child_watch_source->using_pidfd = TRUE; ++ child_watch_source->poll.events = G_IO_IN; ++ g_source_add_poll (source, &child_watch_source->poll); ++ ++ return source; ++ } ++ else ++ { ++ g_debug ("pidfd_open(%" G_PID_FORMAT ") failed with error: %s", ++ pid, g_strerror (errsv)); ++ /* Fall through; likely the kernel isn’t new enough to support pidfd_open() */ ++ } ++#endif /* HAVE_PIDFD */ ++ + G_LOCK (unix_signal_lock); + ref_unix_signal_handler_unlocked (SIGCHLD); + unix_child_watches = g_slist_prepend (unix_child_watches, child_watch_source); + if (waitpid (pid, &child_watch_source->child_status, WNOHANG) > 0) + child_watch_source->child_exited = TRUE; + G_UNLOCK (unix_signal_lock); +-#endif /* G_OS_WIN32 */ ++#endif /* !G_OS_WIN32 */ + + return source; + } +diff --git a/meson.build b/meson.build +index a0223ce5b..1e1bd602c 100644 +--- a/meson.build ++++ b/meson.build +@@ -810,6 +810,20 @@ if cc.links('''#include + glib_conf.set('HAVE_EVENTFD', 1) + endif + ++# Check for pidfd_open(2) ++if cc.links('''#include ++ #include ++ #include ++ #include ++ int main (int argc, char ** argv) { ++ siginfo_t child_info = { 0, }; ++ syscall (SYS_pidfd_open, 0, 0); ++ waitid (P_PIDFD, 0, &child_info, WEXITED | WNOHANG); ++ return 0; ++ }''', name : 'pidfd_open(2) system call') ++ glib_conf.set('HAVE_PIDFD', 1) ++endif ++ + # Check for __uint128_t (gcc) by checking for 128-bit division + uint128_t_src = '''int main() { + static __uint128_t v1 = 100; +-- +2.41.0 + + +From 13c62bc181c6da9f287b737f7a3238e0269b40b3 Mon Sep 17 00:00:00 2001 +From: Christian Hergert +Date: Tue, 2 Aug 2022 12:35:40 -0700 +Subject: [PATCH 2/4] gmain: close pidfd when finalizing GChildWatchSource + +A file-descriptor was created with the introduction of pidfd_getfd() but +nothing is closing it when the source finalizes. The GChildWatchSource is +the creator and consumer of this FD and therefore responsible for closing +it on finalization. + +The pidfd leak was introduced in !2408. + +This fixes issues with Builder where anon_inode:[pidfd] exhaust the +available FD limit for the process. + +Fixes #2708 +--- + glib/gmain.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index e9965f7f6..3ceec61ee 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -5646,7 +5646,11 @@ g_child_watch_finalize (GSource *source) + GChildWatchSource *child_watch_source = (GChildWatchSource *) source; + + if (child_watch_source->using_pidfd) +- return; ++ { ++ if (child_watch_source->poll.fd >= 0) ++ close (child_watch_source->poll.fd); ++ return; ++ } + + G_LOCK (unix_signal_lock); + unix_child_watches = g_slist_remove (unix_child_watches, source); +-- +2.41.0 + + +From 378c72cbe12767b8f6aedc19c7ca46c07aa1ca73 Mon Sep 17 00:00:00 2001 +From: Owen Rafferty +Date: Tue, 12 Jul 2022 20:03:56 -0500 +Subject: [PATCH 3/4] gmain: define non-posix symbols + +--- + glib/gmain.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/glib/gmain.c b/glib/gmain.c +index 3ceec61ee..a2d7bb3ba 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -71,6 +71,12 @@ + #include + #include + #include /* P_PIDFD */ ++#ifndef W_EXITCODE ++#define W_EXITCODE(ret, sig) ((ret) << 8 | (sig)) ++#endif ++#ifndef W_STOPCODE ++#define W_STOPCODE(sig) ((sig) << 8 | 0x7f) ++#endif + #endif /* HAVE_PIDFD */ + + #ifdef G_OS_WIN32 +-- +2.41.0 + + +From aac37188ce26366bd86626700d49cee0cb121472 Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Wed, 21 Dec 2022 12:11:46 +0000 +Subject: [PATCH 4/4] gmain: Define fallback values for siginfo_t constants for + musl +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +musl doesn’t define them itself, presumably because they’re not defined +in POSIX. glibc does define them. Thankfully, the values used in glibc +match the values used internally in other musl macros. + +Define the values as a fallback. As a result of this, we can get rid of +the `g_assert_if_reached()` checks in `siginfo_t_to_wait_status()`. + +This should fix catching signals from a subprocess when built against +musl. + +Signed-off-by: Philip Withnall + +Fixes: #2852 +--- + glib/gmain.c | 18 ++++++++++-------- + 1 file changed, 10 insertions(+), 8 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index a2d7bb3ba..f0cf700c0 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -77,6 +77,16 @@ + #ifndef W_STOPCODE + #define W_STOPCODE(sig) ((sig) << 8 | 0x7f) + #endif ++#ifndef WCOREFLAG ++/* musl doesn’t define WCOREFLAG while glibc does. Unfortunately, there’s no way ++ * to detect we’re building against musl, so just define it and hope. ++ * See https://git.musl-libc.org/cgit/musl/tree/include/sys/wait.h#n51 */ ++#define WCOREFLAG 0x80 ++#endif ++#ifndef __W_CONTINUED ++/* Same as above, for musl */ ++#define __W_CONTINUED 0xffff ++#endif + #endif /* HAVE_PIDFD */ + + #ifdef G_OS_WIN32 +@@ -5411,17 +5421,9 @@ siginfo_t_to_wait_status (const siginfo_t *info) + case CLD_KILLED: + return W_EXITCODE (0, info->si_status); + case CLD_DUMPED: +-#ifdef WCOREFLAG + return W_EXITCODE (0, info->si_status | WCOREFLAG); +-#else +- g_assert_not_reached (); +-#endif + case CLD_CONTINUED: +-#ifdef __W_CONTINUED + return __W_CONTINUED; +-#else +- g_assert_not_reached (); +-#endif + case CLD_STOPPED: + case CLD_TRAPPED: + default: +-- +2.41.0 + diff --git a/SOURCES/3353.patch b/SOURCES/3353.patch new file mode 100644 index 0000000..eb9020a --- /dev/null +++ b/SOURCES/3353.patch @@ -0,0 +1,1078 @@ +From 68f0cd7f3d351da1f011a65e080d8e3f26a31525 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Tue, 28 Mar 2023 14:38:08 +0200 +Subject: [PATCH 1/7] gmain: unify win/unix implementations for child watcher + +Let's move the difference between the win/unix implementations closer to +where the difference is. Thereby, we easier see the two implementations +side by side. Splitting it at a higher layer makes the code harder to +read. + +This is just a preparation for what comes next. +--- + glib/gmain.c | 192 ++++++++++++++++++++++++--------------------------- + 1 file changed, 92 insertions(+), 100 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index f0cf700c0..228737efa 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -446,6 +446,11 @@ static gboolean g_child_watch_dispatch (GSource *source, + GSourceFunc callback, + gpointer user_data); + static void g_child_watch_finalize (GSource *source); ++ ++#ifndef G_OS_WIN32 ++static void unref_unix_signal_handler_unlocked (int signum); ++#endif ++ + #ifdef G_OS_UNIX + static void g_unix_signal_handler (int signum); + static gboolean g_unix_signal_watch_prepare (GSource *source, +@@ -5214,17 +5219,49 @@ g_timeout_add_seconds (guint interval, + + /* Child watch functions */ + +-#ifdef G_OS_WIN32 ++#ifdef HAVE_PIDFD ++static int ++siginfo_t_to_wait_status (const siginfo_t *info) ++{ ++ /* Each of these returns is essentially the inverse of WIFEXITED(), ++ * WIFSIGNALED(), etc. */ ++ switch (info->si_code) ++ { ++ case CLD_EXITED: ++ return W_EXITCODE (info->si_status, 0); ++ case CLD_KILLED: ++ return W_EXITCODE (0, info->si_status); ++ case CLD_DUMPED: ++ return W_EXITCODE (0, info->si_status | WCOREFLAG); ++ case CLD_CONTINUED: ++ return __W_CONTINUED; ++ case CLD_STOPPED: ++ case CLD_TRAPPED: ++ default: ++ return W_STOPCODE (info->si_status); ++ } ++} ++#endif /* HAVE_PIDFD */ + + static gboolean + g_child_watch_prepare (GSource *source, + gint *timeout) + { ++#ifdef G_OS_WIN32 + *timeout = -1; + return FALSE; ++#else /* G_OS_WIN32 */ ++ { ++ GChildWatchSource *child_watch_source; ++ ++ child_watch_source = (GChildWatchSource *) source; ++ ++ return g_atomic_int_get (&child_watch_source->child_exited); ++ } ++#endif /* G_OS_WIN32 */ + } + +-static gboolean ++static gboolean + g_child_watch_check (GSource *source) + { + GChildWatchSource *child_watch_source; +@@ -5232,6 +5269,7 @@ g_child_watch_check (GSource *source) + + child_watch_source = (GChildWatchSource *) source; + ++#ifdef G_OS_WIN32 + child_exited = child_watch_source->poll.revents & G_IO_IN; + + if (child_exited) +@@ -5246,15 +5284,45 @@ g_child_watch_check (GSource *source) + */ + if (!GetExitCodeProcess (child_watch_source->pid, &child_status)) + { +- gchar *emsg = g_win32_error_message (GetLastError ()); +- g_warning (G_STRLOC ": GetExitCodeProcess() failed: %s", emsg); +- g_free (emsg); ++ gchar *emsg = g_win32_error_message (GetLastError ()); ++ g_warning (G_STRLOC ": GetExitCodeProcess() failed: %s", emsg); ++ g_free (emsg); + +- child_watch_source->child_status = -1; +- } ++ child_watch_source->child_status = -1; ++ } + else +- child_watch_source->child_status = child_status; ++ child_watch_source->child_status = child_status; + } ++#else /* G_OS_WIN32 */ ++#ifdef HAVE_PIDFD ++ if (child_watch_source->using_pidfd) ++ { ++ child_exited = child_watch_source->poll.revents & G_IO_IN; ++ ++ if (child_exited) ++ { ++ siginfo_t child_info = { 0, }; ++ ++ /* Get the exit status */ ++ if (waitid (P_PIDFD, child_watch_source->poll.fd, &child_info, WEXITED | WNOHANG) >= 0 && ++ child_info.si_pid != 0) ++ { ++ /* waitid() helpfully provides the wait status in a decomposed ++ * form which is quite useful. Unfortunately we have to report it ++ * to the #GChildWatchFunc as a waitpid()-style platform-specific ++ * wait status, so that the user code in #GChildWatchFunc can then ++ * call WIFEXITED() (etc.) on it. That means re-composing the ++ * status information. */ ++ child_watch_source->child_status = siginfo_t_to_wait_status (&child_info); ++ child_watch_source->child_exited = TRUE; ++ } ++ } ++ ++ return child_exited; ++ } ++#endif /* HAVE_PIDFD */ ++ child_exited = g_atomic_int_get (&child_watch_source->child_exited); ++#endif /* G_OS_WIN32 */ + + return child_exited; + } +@@ -5262,9 +5330,24 @@ g_child_watch_check (GSource *source) + static void + g_child_watch_finalize (GSource *source) + { ++#ifndef G_OS_WIN32 ++ GChildWatchSource *child_watch_source = (GChildWatchSource *) source; ++ ++ if (child_watch_source->using_pidfd) ++ { ++ if (child_watch_source->poll.fd >= 0) ++ close (child_watch_source->poll.fd); ++ return; ++ } ++ ++ G_LOCK (unix_signal_lock); ++ unix_child_watches = g_slist_remove (unix_child_watches, source); ++ unref_unix_signal_handler_unlocked (SIGCHLD); ++ G_UNLOCK (unix_signal_lock); ++#endif /* G_OS_WIN32 */ + } + +-#else /* G_OS_WIN32 */ ++#ifndef G_OS_WIN32 + + static void + wake_source (GSource *source) +@@ -5397,79 +5480,6 @@ dispatch_unix_signals (void) + G_UNLOCK(unix_signal_lock); + } + +-static gboolean +-g_child_watch_prepare (GSource *source, +- gint *timeout) +-{ +- GChildWatchSource *child_watch_source; +- +- child_watch_source = (GChildWatchSource *) source; +- +- return g_atomic_int_get (&child_watch_source->child_exited); +-} +- +-#ifdef HAVE_PIDFD +-static int +-siginfo_t_to_wait_status (const siginfo_t *info) +-{ +- /* Each of these returns is essentially the inverse of WIFEXITED(), +- * WIFSIGNALED(), etc. */ +- switch (info->si_code) +- { +- case CLD_EXITED: +- return W_EXITCODE (info->si_status, 0); +- case CLD_KILLED: +- return W_EXITCODE (0, info->si_status); +- case CLD_DUMPED: +- return W_EXITCODE (0, info->si_status | WCOREFLAG); +- case CLD_CONTINUED: +- return __W_CONTINUED; +- case CLD_STOPPED: +- case CLD_TRAPPED: +- default: +- return W_STOPCODE (info->si_status); +- } +-} +-#endif /* HAVE_PIDFD */ +- +-static gboolean +-g_child_watch_check (GSource *source) +-{ +- GChildWatchSource *child_watch_source; +- +- child_watch_source = (GChildWatchSource *) source; +- +-#ifdef HAVE_PIDFD +- if (child_watch_source->using_pidfd) +- { +- gboolean child_exited = child_watch_source->poll.revents & G_IO_IN; +- +- if (child_exited) +- { +- siginfo_t child_info = { 0, }; +- +- /* Get the exit status */ +- if (waitid (P_PIDFD, child_watch_source->poll.fd, &child_info, WEXITED | WNOHANG) >= 0 && +- child_info.si_pid != 0) +- { +- /* waitid() helpfully provides the wait status in a decomposed +- * form which is quite useful. Unfortunately we have to report it +- * to the #GChildWatchFunc as a waitpid()-style platform-specific +- * wait status, so that the user code in #GChildWatchFunc can then +- * call WIFEXITED() (etc.) on it. That means re-composing the +- * status information. */ +- child_watch_source->child_status = siginfo_t_to_wait_status (&child_info); +- child_watch_source->child_exited = TRUE; +- } +- } +- +- return child_exited; +- } +-#endif /* HAVE_PIDFD */ +- +- return g_atomic_int_get (&child_watch_source->child_exited); +-} +- + static gboolean + g_unix_signal_watch_prepare (GSource *source, + gint *timeout) +@@ -5648,24 +5658,6 @@ g_unix_signal_watch_finalize (GSource *source) + G_UNLOCK (unix_signal_lock); + } + +-static void +-g_child_watch_finalize (GSource *source) +-{ +- GChildWatchSource *child_watch_source = (GChildWatchSource *) source; +- +- if (child_watch_source->using_pidfd) +- { +- if (child_watch_source->poll.fd >= 0) +- close (child_watch_source->poll.fd); +- return; +- } +- +- G_LOCK (unix_signal_lock); +- unix_child_watches = g_slist_remove (unix_child_watches, source); +- unref_unix_signal_handler_unlocked (SIGCHLD); +- G_UNLOCK (unix_signal_lock); +-} +- + #endif /* G_OS_WIN32 */ + + static gboolean +-- +2.41.0 + + +From 984e04a77b29c60f66fd60b1f9690e6ff7880574 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Tue, 28 Mar 2023 15:44:30 +0200 +Subject: [PATCH 2/7] gmain: simplify handling child watchers in + dispatch_unix_signals_unlocked() + +- if a child watch source has "using_pidfd", it is never linked in the + unix_child_watches list. Drop that check. +- replace the deep nested if, with an early "continue" in the loop, + if we detect there is nothing to do. It makes the code easier to + read. +--- + glib/gmain.c | 39 +++++++++++++++++++-------------------- + 1 file changed, 19 insertions(+), 20 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index 228737efa..8a030a409 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -5430,31 +5430,30 @@ dispatch_unix_signals_unlocked (void) + for (node = unix_child_watches; node; node = node->next) + { + GChildWatchSource *source = node->data; ++ pid_t pid; + +- if (!source->using_pidfd && +- !g_atomic_int_get (&source->child_exited)) ++ if (g_atomic_int_get (&source->child_exited)) ++ continue; ++ ++ do + { +- pid_t pid; +- do +- { +- g_assert (source->pid > 0); ++ g_assert (source->pid > 0); + +- pid = waitpid (source->pid, &source->child_status, WNOHANG); +- if (pid > 0) +- { +- g_atomic_int_set (&source->child_exited, TRUE); +- wake_source ((GSource *) source); +- } +- else if (pid == -1 && errno == ECHILD) +- { +- g_warning ("GChildWatchSource: Exit status of a child process was requested but ECHILD was received by waitpid(). See the documentation of g_child_watch_source_new() for possible causes."); +- source->child_status = 0; +- g_atomic_int_set (&source->child_exited, TRUE); +- wake_source ((GSource *) source); +- } ++ pid = waitpid (source->pid, &source->child_status, WNOHANG); ++ if (pid > 0) ++ { ++ g_atomic_int_set (&source->child_exited, TRUE); ++ wake_source ((GSource *) source); ++ } ++ else if (pid == -1 && errno == ECHILD) ++ { ++ g_warning ("GChildWatchSource: Exit status of a child process was requested but ECHILD was received by waitpid(). See the documentation of g_child_watch_source_new() for possible causes."); ++ source->child_status = 0; ++ g_atomic_int_set (&source->child_exited, TRUE); ++ wake_source ((GSource *) source); + } +- while (pid == -1 && errno == EINTR); + } ++ while (pid == -1 && errno == EINTR); + } + } + +-- +2.41.0 + + +From aaac91a86288e103f7f413b9d075acc803178da9 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Wed, 17 May 2023 08:04:02 +0200 +Subject: [PATCH 3/7] gmain: remove unnecessary initialization of + source_timeout in g_main_context_prepare_unlocked() + +Note that the variable source_timeout is already initialized upon +definition, at the beginning of the block. + +It's easy to see, that no code changes the variable between the variable +definition, and the place where it was initialized. It was thus +unnecessary. + +It's not about dropping the unnecessary code (the compiler could do that +just fine too). It's that there is the other branch of the "if/else", where +the variable is also not initialized. But the other branch also requires +that the variable is in fact initialized to -1, because prepare() +callbacks are free not to explicitly set the output value. So both +branches require the variable to be initialized to -1, but only one of +them did. This poses unnecessary questions about whether anything is +wrong. Avoid that by dropping the redundant code. +--- + glib/gmain.c | 5 +---- + 1 file changed, 1 insertion(+), 4 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index 8a030a409..28fbcc015 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -3695,10 +3695,7 @@ g_main_context_prepare (GMainContext *context, + context->in_check_or_prepare--; + } + else +- { +- source_timeout = -1; +- result = FALSE; +- } ++ result = FALSE; + + if (result == FALSE && source->priv->ready_time != -1) + { +-- +2.41.0 + + +From b71ae65f14843cc09819e5b482667e5c941a27af Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Wed, 17 May 2023 08:15:16 +0200 +Subject: [PATCH 4/7] gmain: remove unnecessary initialization of *timeout in + prepare() callbacks + +Note that the prepare callback only has one caller, which pre-initializes +the timeout argument to -1. That may be an implementation detail and not +publicly promised, but it wouldn't make sense to do it any other way in +the caller. + +Also, note that g_unix_signal_watch_prepare() and the UNIX branch of +g_child_watch_prepare() already relied on that. +--- + gio/gsocket.c | 2 -- + glib/giounix.c | 2 -- + glib/giowin32.c | 2 -- + glib/gmain.c | 1 - + glib/tests/mainloop.c | 2 ++ + 5 files changed, 2 insertions(+), 7 deletions(-) + +diff --git a/gio/gsocket.c b/gio/gsocket.c +index f39a568b3..c624eb1ae 100644 +--- a/gio/gsocket.c ++++ b/gio/gsocket.c +@@ -3955,8 +3955,6 @@ socket_source_prepare (GSource *source, + { + GSocketSource *socket_source = (GSocketSource *)source; + +- *timeout = -1; +- + #ifdef G_OS_WIN32 + if ((socket_source->pollfd.revents & G_IO_NVAL) != 0) + return TRUE; +diff --git a/glib/giounix.c b/glib/giounix.c +index b86d79db7..94b33253f 100644 +--- a/glib/giounix.c ++++ b/glib/giounix.c +@@ -129,8 +129,6 @@ g_io_unix_prepare (GSource *source, + GIOUnixWatch *watch = (GIOUnixWatch *)source; + GIOCondition buffer_condition = g_io_channel_get_buffer_condition (watch->channel); + +- *timeout = -1; +- + /* Only return TRUE here if _all_ bits in watch->condition will be set + */ + return ((watch->condition & buffer_condition) == watch->condition); +diff --git a/glib/giowin32.c b/glib/giowin32.c +index b0b6c3d85..e4b171b0d 100644 +--- a/glib/giowin32.c ++++ b/glib/giowin32.c +@@ -707,8 +707,6 @@ g_io_win32_prepare (GSource *source, + GIOWin32Channel *channel = (GIOWin32Channel *)watch->channel; + int event_mask; + +- *timeout = -1; +- + if (channel->debug) + g_print ("g_io_win32_prepare: source=%p channel=%p", source, channel); + +diff --git a/glib/gmain.c b/glib/gmain.c +index 28fbcc015..aec04314c 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -5245,7 +5245,6 @@ g_child_watch_prepare (GSource *source, + gint *timeout) + { + #ifdef G_OS_WIN32 +- *timeout = -1; + return FALSE; + #else /* G_OS_WIN32 */ + { +diff --git a/glib/tests/mainloop.c b/glib/tests/mainloop.c +index d43b2cf08..a7c5b33d1 100644 +--- a/glib/tests/mainloop.c ++++ b/glib/tests/mainloop.c +@@ -34,6 +34,8 @@ cb (gpointer data) + static gboolean + prepare (GSource *source, gint *time) + { ++ g_assert_nonnull (time); ++ g_assert_cmpint (*time, ==, -1); + return FALSE; + } + static gboolean +-- +2.41.0 + + +From ccbebd3bd2d9163da3e2ad7f213ded04cd92136d Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Tue, 28 Mar 2023 14:30:58 +0200 +Subject: [PATCH 5/7] gmain: fix race with waitpid() and child watcher sources + +GChildWatchSource uses waitpid(), among pidfd and GetExitCodeProcess(). +It thus only works for child processes which the user must ensure to +exist and not being reaped yet. Also, the user must not kill() the PID +after the child process is reaped and must not race kill() against +waitpid(). Also, the user must not call waitpid()/kill() after the child +process is reaped. + +Previously, GChildWatchSource would call waitpid() already when adding +the source (g_child_watch_source_new()) and from the worker thread +(dispatch_unix_signals_unlocked()). That is racy: + +- if a child watcher is attached and did not yet fire, you cannot call + kill() on the PID without racing against the PID being reaped on the + worker thread. That would then lead to ESRCH or even worse, killing + the wrong process. + +- if you g_source_destroy() the source that didn't fire yet, the user + doesn't know whether the PID was reaped in the background. Any + subsequent kill()/waitpid() may fail with ESRCH/ECHILD or even address + the wrong process. + +The race is most visible on Unix without pidfd support, because then the +process gets reaped on the worker thread or during g_child_watch_source_new(). +But it's also with Windows and pidfd, because we would have waited for +the process in g_child_watch_check(), where other callbacks could fire +between reaping the process status and emitting the source's callback. + +Fix all that by calling waitpid() right before dispatching the callback. +--- + glib/gmain.c | 209 +++++++++++++++++++++++++++------------------- + glib/tests/unix.c | 93 +++++++++++++++++++++ + 2 files changed, 218 insertions(+), 84 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index aec04314c..d1ab421fb 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -350,12 +350,11 @@ struct _GChildWatchSource + { + GSource source; + GPid pid; +- gint child_status; + /* @poll is always used on Windows, and used on Unix iff @using_pidfd is set: */ + GPollFD poll; + #ifndef G_OS_WIN32 +- gboolean child_exited; /* (atomic); not used iff @using_pidfd is set */ +- gboolean using_pidfd; ++ gboolean child_maybe_exited; /* (atomic) */ ++ gboolean using_pidfd; + #endif /* G_OS_WIN32 */ + }; + +@@ -5238,7 +5237,7 @@ siginfo_t_to_wait_status (const siginfo_t *info) + return W_STOPCODE (info->si_status); + } + } +-#endif /* HAVE_PIDFD */ ++#endif /* HAVE_PIDFD */ + + static gboolean + g_child_watch_prepare (GSource *source, +@@ -5246,19 +5245,19 @@ g_child_watch_prepare (GSource *source, + { + #ifdef G_OS_WIN32 + return FALSE; +-#else /* G_OS_WIN32 */ ++#else /* G_OS_WIN32 */ + { + GChildWatchSource *child_watch_source; + + child_watch_source = (GChildWatchSource *) source; + +- return g_atomic_int_get (&child_watch_source->child_exited); ++ return !child_watch_source->using_pidfd && g_atomic_int_get (&child_watch_source->child_maybe_exited); + } + #endif /* G_OS_WIN32 */ + } + + static gboolean +-g_child_watch_check (GSource *source) ++g_child_watch_check (GSource *source) + { + GChildWatchSource *child_watch_source; + gboolean child_exited; +@@ -5267,57 +5266,15 @@ g_child_watch_check (GSource *source) + + #ifdef G_OS_WIN32 + child_exited = child_watch_source->poll.revents & G_IO_IN; +- +- if (child_exited) +- { +- DWORD child_status; +- +- /* +- * Note: We do _not_ check for the special value of STILL_ACTIVE +- * since we know that the process has exited and doing so runs into +- * problems if the child process "happens to return STILL_ACTIVE(259)" +- * as Microsoft's Platform SDK puts it. +- */ +- if (!GetExitCodeProcess (child_watch_source->pid, &child_status)) +- { +- gchar *emsg = g_win32_error_message (GetLastError ()); +- g_warning (G_STRLOC ": GetExitCodeProcess() failed: %s", emsg); +- g_free (emsg); +- +- child_watch_source->child_status = -1; +- } +- else +- child_watch_source->child_status = child_status; +- } + #else /* G_OS_WIN32 */ + #ifdef HAVE_PIDFD + if (child_watch_source->using_pidfd) + { + child_exited = child_watch_source->poll.revents & G_IO_IN; +- +- if (child_exited) +- { +- siginfo_t child_info = { 0, }; +- +- /* Get the exit status */ +- if (waitid (P_PIDFD, child_watch_source->poll.fd, &child_info, WEXITED | WNOHANG) >= 0 && +- child_info.si_pid != 0) +- { +- /* waitid() helpfully provides the wait status in a decomposed +- * form which is quite useful. Unfortunately we have to report it +- * to the #GChildWatchFunc as a waitpid()-style platform-specific +- * wait status, so that the user code in #GChildWatchFunc can then +- * call WIFEXITED() (etc.) on it. That means re-composing the +- * status information. */ +- child_watch_source->child_status = siginfo_t_to_wait_status (&child_info); +- child_watch_source->child_exited = TRUE; +- } +- } +- + return child_exited; + } +-#endif /* HAVE_PIDFD */ +- child_exited = g_atomic_int_get (&child_watch_source->child_exited); ++#endif /* HAVE_PIDFD */ ++ child_exited = g_atomic_int_get (&child_watch_source->child_maybe_exited); + #endif /* G_OS_WIN32 */ + + return child_exited; +@@ -5426,30 +5383,9 @@ dispatch_unix_signals_unlocked (void) + for (node = unix_child_watches; node; node = node->next) + { + GChildWatchSource *source = node->data; +- pid_t pid; +- +- if (g_atomic_int_get (&source->child_exited)) +- continue; + +- do +- { +- g_assert (source->pid > 0); +- +- pid = waitpid (source->pid, &source->child_status, WNOHANG); +- if (pid > 0) +- { +- g_atomic_int_set (&source->child_exited, TRUE); +- wake_source ((GSource *) source); +- } +- else if (pid == -1 && errno == ECHILD) +- { +- g_warning ("GChildWatchSource: Exit status of a child process was requested but ECHILD was received by waitpid(). See the documentation of g_child_watch_source_new() for possible causes."); +- source->child_status = 0; +- g_atomic_int_set (&source->child_exited, TRUE); +- wake_source ((GSource *) source); +- } +- } +- while (pid == -1 && errno == EINTR); ++ if (g_atomic_int_compare_and_exchange (&source->child_maybe_exited, FALSE, TRUE)) ++ wake_source ((GSource *) source); + } + } + +@@ -5662,9 +5598,106 @@ g_child_watch_dispatch (GSource *source, + { + GChildWatchSource *child_watch_source; + GChildWatchFunc child_watch_callback = (GChildWatchFunc) callback; ++ int wait_status; + + child_watch_source = (GChildWatchSource *) source; + ++ /* We only (try to) reap the child process right before dispatching the callback. ++ * That way, the caller can rely that the process is there until the callback ++ * is invoked; or, if the caller calls g_source_destroy() without the callback ++ * being dispatched, the process is still not reaped. */ ++ ++#ifdef G_OS_WIN32 ++ { ++ DWORD child_status; ++ ++ /* ++ * Note: We do _not_ check for the special value of STILL_ACTIVE ++ * since we know that the process has exited and doing so runs into ++ * problems if the child process "happens to return STILL_ACTIVE(259)" ++ * as Microsoft's Platform SDK puts it. ++ */ ++ if (!GetExitCodeProcess (child_watch_source->pid, &child_status)) ++ { ++ gchar *emsg = g_win32_error_message (GetLastError ()); ++ g_warning (G_STRLOC ": GetExitCodeProcess() failed: %s", emsg); ++ g_free (emsg); ++ ++ /* Unknown error. We got signaled that the process might be exited, ++ * but now we failed to reap it? Assume the process is gone and proceed. */ ++ wait_status = -1; ++ } ++ else ++ wait_status = child_status; ++ } ++#else /* G_OS_WIN32 */ ++ { ++ gboolean child_exited = FALSE; ++ ++ wait_status = -1; ++ ++#ifdef HAVE_PIDFD ++ if (child_watch_source->using_pidfd) ++ { ++ siginfo_t child_info = { ++ 0, ++ }; ++ ++ /* Get the exit status */ ++ if (waitid (P_PIDFD, child_watch_source->poll.fd, &child_info, WEXITED | WNOHANG) >= 0 && ++ child_info.si_pid != 0) ++ { ++ /* waitid() helpfully provides the wait status in a decomposed ++ * form which is quite useful. Unfortunately we have to report it ++ * to the #GChildWatchFunc as a waitpid()-style platform-specific ++ * wait status, so that the user code in #GChildWatchFunc can then ++ * call WIFEXITED() (etc.) on it. That means re-composing the ++ * status information. */ ++ wait_status = siginfo_t_to_wait_status (&child_info); ++ } ++ else ++ { ++ /* Unknown error. We got signaled that the process might be exited, ++ * but now we failed to reap it? Assume the process is gone and proceed. */ ++ g_warning (G_STRLOC ": pidfd signaled ready but failed"); ++ } ++ child_exited = TRUE; ++ } ++#endif /* HAVE_PIDFD*/ ++ ++ if (!child_exited) ++ { ++ pid_t pid; ++ int wstatus; ++ ++ waitpid_again: ++ ++ /* We must reset the flag before waitpid(). Otherwise, there would be a ++ * race. */ ++ g_atomic_int_set (&child_watch_source->child_maybe_exited, FALSE); ++ ++ pid = waitpid (child_watch_source->pid, &wstatus, WNOHANG); ++ ++ if (pid == 0) ++ { ++ /* Not exited yet. Wait longer. */ ++ return TRUE; ++ } ++ ++ if (pid > 0) ++ wait_status = wstatus; ++ else if (errno == ECHILD) ++ g_warning ("GChildWatchSource: Exit status of a child process was requested but ECHILD was received by waitpid(). See the documentation of g_child_watch_source_new() for possible causes."); ++ else if (errno == EINTR) ++ goto waitpid_again; ++ else ++ { ++ /* Unexpected error. Whatever happened, we are done waiting for this child. */ ++ } ++ } ++ } ++#endif /* G_OS_WIN32 */ ++ + if (!callback) + { + g_warning ("Child watch source dispatched without callback. " +@@ -5672,7 +5705,7 @@ g_child_watch_dispatch (GSource *source, + return FALSE; + } + +- (child_watch_callback) (child_watch_source->pid, child_watch_source->child_status, user_data); ++ (child_watch_callback) (child_watch_source->pid, wait_status, user_data); + + /* We never keep a child watch source around as the child is gone */ + return FALSE; +@@ -5731,6 +5764,14 @@ g_unix_signal_handler (int signum) + * mechanism, including `waitpid(pid, ...)` or a second child-watch + * source for the same @pid + * * the application must not ignore `SIGCHLD` ++ * * Before 2.78, the application could not send a signal (`kill()`) to the ++ * watched @pid in a race free manner. Since 2.78, you can do that while the ++ * associated #GMainContext is acquired. ++ * * Before 2.78, even after destroying the #GSource, you could not ++ * be sure that @pid wasn't already reaped. Hence, it was also not ++ * safe to `kill()` or `waitpid()` on the process ID after the child watch ++ * source was gone. Destroying the source before it fired made it ++ * impossible to reliably reap the process. + * + * If any of those conditions are not met, this and related APIs will + * not work correctly. This can often be diagnosed via a GLib warning +@@ -5791,19 +5832,19 @@ g_child_watch_source_new (GPid pid) + + return source; + } +- else +- { +- g_debug ("pidfd_open(%" G_PID_FORMAT ") failed with error: %s", +- pid, g_strerror (errsv)); +- /* Fall through; likely the kernel isn’t new enough to support pidfd_open() */ +- } +-#endif /* HAVE_PIDFD */ ++ ++ g_debug ("pidfd_open(%" G_PID_FORMAT ") failed with error: %s", ++ pid, g_strerror (errsv)); ++ /* Fall through; likely the kernel isn’t new enough to support pidfd_open() */ ++#endif /* HAVE_PIDFD */ ++ ++ /* We can do that without atomic, as the source is not yet added in ++ * unix_child_watches (which we do next under a lock). */ ++ child_watch_source->child_maybe_exited = TRUE; + + G_LOCK (unix_signal_lock); + ref_unix_signal_handler_unlocked (SIGCHLD); + unix_child_watches = g_slist_prepend (unix_child_watches, child_watch_source); +- if (waitpid (pid, &child_watch_source->child_status, WNOHANG) > 0) +- child_watch_source->child_exited = TRUE; + G_UNLOCK (unix_signal_lock); + #endif /* !G_OS_WIN32 */ + +diff --git a/glib/tests/unix.c b/glib/tests/unix.c +index 7639d066a..6f40ff893 100644 +--- a/glib/tests/unix.c ++++ b/glib/tests/unix.c +@@ -330,6 +330,99 @@ test_get_passwd_entry_nonexistent (void) + g_clear_error (&local_error); + } + ++static void ++_child_wait_watch_cb (GPid pid, ++ gint wait_status, ++ gpointer user_data) ++{ ++ gboolean *p_got_callback = user_data; ++ ++ g_assert_nonnull (p_got_callback); ++ g_assert_false (*p_got_callback); ++ *p_got_callback = TRUE; ++} ++ ++static void ++test_child_wait (void) ++{ ++ gboolean r; ++ GPid pid; ++ guint id; ++ pid_t pid2; ++ int wstatus; ++ gboolean got_callback = FALSE; ++ gboolean iterate_maincontext = g_test_rand_bit (); ++ char **argv; ++ int errsv; ++ ++ /* - We spawn a trivial child process that exits after a short time. ++ * - We schedule a g_child_watch_add() ++ * - we may iterate the GMainContext a bit. Randomly we either get the ++ * child-watcher callback or not. ++ * - if we didn't get the callback, we g_source_remove() the child watcher. ++ * ++ * Afterwards, if the callback didn't fire, we check that we are able to waitpid() ++ * on the process ourselves. Of course, if the child watcher notified, the waitpid() ++ * will fail with ECHILD. ++ */ ++ ++ argv = g_test_rand_bit () ? ((char *[]){ "/bin/sleep", "0.05", NULL }) : ((char *[]){ "/bin/true", NULL }); ++ ++ r = g_spawn_async (NULL, ++ argv, ++ NULL, ++ G_SPAWN_DO_NOT_REAP_CHILD, ++ NULL, ++ NULL, ++ &pid, ++ NULL); ++ if (!r) ++ { ++ /* Some odd system without /bin/sleep? Skip the test. */ ++ g_test_skip ("failure to spawn test process in test_child_wait()"); ++ return; ++ } ++ ++ g_assert_cmpint (pid, >=, 1); ++ ++ if (g_test_rand_bit ()) ++ g_usleep (g_test_rand_int_range (0, (G_USEC_PER_SEC / 10))); ++ ++ id = g_child_watch_add (pid, _child_wait_watch_cb, &got_callback); ++ ++ if (g_test_rand_bit ()) ++ g_usleep (g_test_rand_int_range (0, (G_USEC_PER_SEC / 10))); ++ ++ if (iterate_maincontext) ++ { ++ gint64 start_usec = g_get_monotonic_time (); ++ gint64 end_usec = start_usec + g_test_rand_int_range (0, (G_USEC_PER_SEC / 10)); ++ ++ while (!got_callback && g_get_monotonic_time () < end_usec) ++ g_main_context_iteration (NULL, FALSE); ++ } ++ ++ if (!got_callback) ++ g_source_remove (id); ++ ++ errno = 0; ++ pid2 = waitpid (pid, &wstatus, 0); ++ errsv = errno; ++ if (got_callback) ++ { ++ g_assert_true (iterate_maincontext); ++ g_assert_cmpint (errsv, ==, ECHILD); ++ g_assert_cmpint (pid2, <, 0); ++ } ++ else ++ { ++ g_assert_cmpint (errsv, ==, 0); ++ g_assert_cmpint (pid2, ==, pid); ++ g_assert_true (WIFEXITED (wstatus)); ++ g_assert_cmpint (WEXITSTATUS (wstatus), ==, 0); ++ } ++} ++ + int + main (int argc, + char *argv[]) +-- +2.41.0 + + +From ad18d2dad0b7105a379e1b2feae653bf30418397 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Tue, 28 Mar 2023 19:53:02 +0200 +Subject: [PATCH 6/7] gmain: drop redundant using_pidfd field from + GChildWatchSource + +It's redundant, which leads to impossible code like: + + if (child_watch_source->using_pidfd) + { + if (child_watch_source->poll.fd >= 0) + close (child_watch_source->poll.fd); +--- + glib/gmain.c | 23 ++++++++++++----------- + 1 file changed, 12 insertions(+), 11 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index d1ab421fb..3813b032a 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -350,11 +350,11 @@ struct _GChildWatchSource + { + GSource source; + GPid pid; +- /* @poll is always used on Windows, and used on Unix iff @using_pidfd is set: */ ++ /* @poll is always used on Windows. ++ * On Unix, poll.fd will be negative if PIDFD is unavailable. */ + GPollFD poll; + #ifndef G_OS_WIN32 + gboolean child_maybe_exited; /* (atomic) */ +- gboolean using_pidfd; + #endif /* G_OS_WIN32 */ + }; + +@@ -5251,7 +5251,10 @@ g_child_watch_prepare (GSource *source, + + child_watch_source = (GChildWatchSource *) source; + +- return !child_watch_source->using_pidfd && g_atomic_int_get (&child_watch_source->child_maybe_exited); ++ if (child_watch_source->poll.fd >= 0) ++ return FALSE; ++ ++ return g_atomic_int_get (&child_watch_source->child_maybe_exited); + } + #endif /* G_OS_WIN32 */ + } +@@ -5268,7 +5271,7 @@ g_child_watch_check (GSource *source) + child_exited = child_watch_source->poll.revents & G_IO_IN; + #else /* G_OS_WIN32 */ + #ifdef HAVE_PIDFD +- if (child_watch_source->using_pidfd) ++ if (child_watch_source->poll.fd >= 0) + { + child_exited = child_watch_source->poll.revents & G_IO_IN; + return child_exited; +@@ -5286,10 +5289,9 @@ g_child_watch_finalize (GSource *source) + #ifndef G_OS_WIN32 + GChildWatchSource *child_watch_source = (GChildWatchSource *) source; + +- if (child_watch_source->using_pidfd) ++ if (child_watch_source->poll.fd >= 0) + { +- if (child_watch_source->poll.fd >= 0) +- close (child_watch_source->poll.fd); ++ close (child_watch_source->poll.fd); + return; + } + +@@ -5637,7 +5639,7 @@ g_child_watch_dispatch (GSource *source, + wait_status = -1; + + #ifdef HAVE_PIDFD +- if (child_watch_source->using_pidfd) ++ if (child_watch_source->poll.fd >= 0) + { + siginfo_t child_info = { + 0, +@@ -5822,17 +5824,15 @@ g_child_watch_source_new (GPid pid) + * better than SIGCHLD. + */ + child_watch_source->poll.fd = (int) syscall (SYS_pidfd_open, pid, 0); +- errsv = errno; + + if (child_watch_source->poll.fd >= 0) + { +- child_watch_source->using_pidfd = TRUE; + child_watch_source->poll.events = G_IO_IN; + g_source_add_poll (source, &child_watch_source->poll); +- + return source; + } + ++ errsv = errno; + g_debug ("pidfd_open(%" G_PID_FORMAT ") failed with error: %s", + pid, g_strerror (errsv)); + /* Fall through; likely the kernel isn’t new enough to support pidfd_open() */ +@@ -5841,6 +5841,7 @@ g_child_watch_source_new (GPid pid) + /* We can do that without atomic, as the source is not yet added in + * unix_child_watches (which we do next under a lock). */ + child_watch_source->child_maybe_exited = TRUE; ++ child_watch_source->poll.fd = -1; + + G_LOCK (unix_signal_lock); + ref_unix_signal_handler_unlocked (SIGCHLD); +-- +2.41.0 + + +From bbdcc6e72ac97e577486aeb1baad0060ad8e1cd6 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Wed, 29 Mar 2023 07:51:26 +0200 +Subject: [PATCH 7/7] gmain: ensure boolean value in g_child_watch_check() is + strictly 0 or 1 + +No problem in practice, but it seems nice to ensure that a gboolean is +always either FALSE or TRUE. +--- + glib/gmain.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/glib/gmain.c b/glib/gmain.c +index 3813b032a..b6ffc6dd0 100644 +--- a/glib/gmain.c ++++ b/glib/gmain.c +@@ -5268,12 +5268,12 @@ g_child_watch_check (GSource *source) + child_watch_source = (GChildWatchSource *) source; + + #ifdef G_OS_WIN32 +- child_exited = child_watch_source->poll.revents & G_IO_IN; ++ child_exited = !!(child_watch_source->poll.revents & G_IO_IN); + #else /* G_OS_WIN32 */ + #ifdef HAVE_PIDFD + if (child_watch_source->poll.fd >= 0) + { +- child_exited = child_watch_source->poll.revents & G_IO_IN; ++ child_exited = !!(child_watch_source->poll.revents & G_IO_IN); + return child_exited; + } + #endif /* HAVE_PIDFD */ +-- +2.41.0 + diff --git a/SOURCES/3845.patch b/SOURCES/3845.patch new file mode 100644 index 0000000..8cd6b01 --- /dev/null +++ b/SOURCES/3845.patch @@ -0,0 +1,195 @@ +From 37e323f1d16720d662611866cde567b1d2a01d48 Mon Sep 17 00:00:00 2001 +From: Ondrej Holy +Date: Mon, 22 Jan 2024 15:29:37 +0100 +Subject: [PATCH 1/2] gunixmounts: Use libmnt_monitor API for monitoring + +The `GUnixMountMonitor` object implements monitoring on its own currently. +Only the `/proc/mounts` file changes are monitored. It is not aware of the +`/run/mount/utab` file changes. This file contains the userspace mount +options (e.g. `x-gvfs-notrash`, `x-gvfs-hide`) among others. There is a +problem when `/sbin/mount.` (e.g. `mount.nfs`) helper programs are +used. In that case, the `/run/mount/utab` file is updated later than the +`/proc/mounts` file and thus the `GUnixMountMonitor` clients (e.g. +`gvfs-udisks2-volume-monitor`, `gvfsd-trash`) don't see the userspace +options until the next `mount-changed` signal. Let's use the `libmnt_monitor` +API for monitoring instead and emit the `mount-changed` signal also when the +`/run/mount/utab` file is changed. + +Related: https://issues.redhat.com/browse/RHEL-14607 +Related: https://github.com/util-linux/util-linux/pull/2607 +--- + gio/gunixmounts.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++- + 1 file changed, 71 insertions(+), 1 deletion(-) + +diff --git a/gio/gunixmounts.c b/gio/gunixmounts.c +index 32b936259..e11b34a7d 100644 +--- a/gio/gunixmounts.c ++++ b/gio/gunixmounts.c +@@ -202,6 +202,11 @@ static GSource *proc_mounts_watch_source; + #define endmntent(f) fclose(f) + #endif + ++#ifdef HAVE_LIBMOUNT ++/* Protected by proc_mounts_source lock */ ++static struct libmnt_monitor *proc_mounts_monitor = NULL; ++#endif ++ + static gboolean + is_in (const char *value, const char *set[]) + { +@@ -1859,7 +1864,36 @@ proc_mounts_changed (GIOChannel *channel, + GIOCondition cond, + gpointer user_data) + { ++ gboolean has_changed = FALSE; ++ ++#ifdef HAVE_LIBMOUNT ++ if (cond & G_IO_IN) ++ { ++ G_LOCK (proc_mounts_source); ++ if (proc_mounts_monitor != NULL) ++ { ++ int ret; ++ ++ /* The mnt_monitor_next_change function needs to be used to avoid false-positives. */ ++ ret = mnt_monitor_next_change (proc_mounts_monitor, NULL, NULL); ++ if (ret == 0) ++ { ++ has_changed = TRUE; ++ ret = mnt_monitor_event_cleanup (proc_mounts_monitor); ++ } ++ ++ if (ret < 0) ++ g_debug ("mnt_monitor_next_change failed: %s", g_strerror (-ret)); ++ } ++ G_UNLOCK (proc_mounts_source); ++ } ++ ++#else + if (cond & G_IO_ERR) ++ has_changed = TRUE; ++#endif ++ ++ if (has_changed) + { + G_LOCK (proc_mounts_source); + mount_poller_time = (guint64) g_get_monotonic_time (); +@@ -1924,6 +1958,10 @@ mount_monitor_stop (void) + g_source_destroy (proc_mounts_watch_source); + proc_mounts_watch_source = NULL; + } ++ ++#ifdef HAVE_LIBMOUNT ++ g_clear_pointer (&proc_mounts_monitor, mnt_unref_monitor); ++#endif + G_UNLOCK (proc_mounts_source); + + if (mtab_monitor) +@@ -1965,9 +2003,37 @@ mount_monitor_start (void) + */ + if (g_str_has_prefix (mtab_path, "/proc/")) + { +- GIOChannel *proc_mounts_channel; ++ GIOChannel *proc_mounts_channel = NULL; + GError *error = NULL; ++#ifdef HAVE_LIBMOUNT ++ int ret; ++ ++ G_LOCK (proc_mounts_source); ++ ++ proc_mounts_monitor = mnt_new_monitor (); ++ ret = mnt_monitor_enable_kernel (proc_mounts_monitor, TRUE); ++ if (ret < 0) ++ g_warning ("mnt_monitor_enable_kernel failed: %s", g_strerror (-ret)); ++ ++ ret = mnt_monitor_enable_userspace (proc_mounts_monitor, TRUE, NULL); ++ if (ret < 0) ++ g_warning ("mnt_monitor_enable_userspace failed: %s", g_strerror (-ret)); ++ ++ ret = mnt_monitor_get_fd (proc_mounts_monitor); ++ if (ret >= 0) ++ { ++ proc_mounts_channel = g_io_channel_unix_new (ret); ++ } ++ else ++ { ++ g_set_error_literal (&error, G_IO_ERROR, g_io_error_from_errno (-ret), ++ g_strerror (-ret)); ++ } ++ ++ G_UNLOCK (proc_mounts_source); ++#else + proc_mounts_channel = g_io_channel_new_file (mtab_path, "r", &error); ++#endif + if (proc_mounts_channel == NULL) + { + g_warning ("Error creating IO channel for %s: %s (%s, %d)", mtab_path, +@@ -1978,7 +2044,11 @@ mount_monitor_start (void) + { + G_LOCK (proc_mounts_source); + ++#ifdef HAVE_LIBMOUNT ++ proc_mounts_watch_source = g_io_create_watch (proc_mounts_channel, G_IO_IN); ++#else + proc_mounts_watch_source = g_io_create_watch (proc_mounts_channel, G_IO_ERR); ++#endif + mount_poller_time = (guint64) g_get_monotonic_time (); + g_source_set_callback (proc_mounts_watch_source, + (GSourceFunc) proc_mounts_changed, +-- +2.43.0 + + +From bb7d6b8fcef36af5452071c8758f89955888469a Mon Sep 17 00:00:00 2001 +From: Ondrej Holy +Date: Wed, 31 Jan 2024 13:35:39 +0100 +Subject: [PATCH 2/2] gunixmounts: Use mnt_monitor_veil_kernel option + +The previous commit enabled the `/run/mount/utab` monitoring. The problem +is that the `mount-changed` signal can be emitted twice for one mount. One +for the `/proc/mounts` file change and another one for the `/run/media/utab` +file change. This is still not ideal because e.g. the `GMount` objects for +mounts with the `x-gvfs-hide` option are added and immediately removed. +Let's enable the `mnt_monitor_veil_kernel` option to avoid this. + +Related: https://github.com/util-linux/util-linux/pull/2725 +--- + gio/gunixmounts.c | 6 ++++++ + meson.build | 4 ++++ + 2 files changed, 10 insertions(+) + +diff --git a/gio/gunixmounts.c b/gio/gunixmounts.c +index e11b34a7d..6abe87414 100644 +--- a/gio/gunixmounts.c ++++ b/gio/gunixmounts.c +@@ -2019,6 +2019,12 @@ mount_monitor_start (void) + if (ret < 0) + g_warning ("mnt_monitor_enable_userspace failed: %s", g_strerror (-ret)); + ++#ifdef HAVE_MNT_MONITOR_VEIL_KERNEL ++ ret = mnt_monitor_veil_kernel (proc_mounts_monitor, TRUE); ++ if (ret < 0) ++ g_warning ("mnt_monitor_veil_kernel failed: %s", g_strerror (-ret)); ++#endif ++ + ret = mnt_monitor_get_fd (proc_mounts_monitor); + if (ret >= 0) + { +diff --git a/meson.build b/meson.build +index a0502fe69..159703557 100644 +--- a/meson.build ++++ b/meson.build +@@ -2102,6 +2102,10 @@ libmount_dep = [] + if host_system == 'linux' + libmount_dep = dependency('mount', version : '>=2.23', required : get_option('libmount')) + glib_conf.set('HAVE_LIBMOUNT', libmount_dep.found()) ++ ++ if libmount_dep.found() and cc.has_function('mnt_monitor_veil_kernel', dependencies: libmount_dep) ++ glib_conf.set('HAVE_MNT_MONITOR_VEIL_KERNEL', 1) ++ endif + endif + + # gnutls is used optionally by GHmac +-- +2.43.0 + diff --git a/SPECS/glib2.spec b/SPECS/glib2.spec index d871cca..36f9c43 100644 --- a/SPECS/glib2.spec +++ b/SPECS/glib2.spec @@ -1,6 +1,6 @@ Name: glib2 Version: 2.68.4 -Release: 11%{?dist} +Release: 14%{?dist} Summary: A library of handy utility functions License: LGPLv2+ @@ -43,6 +43,17 @@ Patch: 3163.patch Patch: 2826.patch Patch: 3272.patch +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2408 +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2816 +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2847 +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3158 +Patch: 2408.patch +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3353 +Patch: 3353.patch + +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3845 +Patch: 3845.patch + BuildRequires: chrpath BuildRequires: gcc BuildRequires: gcc-c++ @@ -258,6 +269,18 @@ glib-compile-schemas %{_datadir}/glib-2.0/schemas &> /dev/null || : %{_datadir}/installed-tests %changelog +* Wed Feb 21 2024 Michael Catanzaro - 2.68.4-14 +- Rebuild against newer util-linux for libmnt changes +- Resolves: RHEL-23637 + +* Thu Feb 01 2024 Michael Catanzaro - 2.68.4-13 +- Backport GUnixMountMonitor port to libmnt_monitor +- Resolves: RHEL-23637 + +* Fri Nov 03 2023 Michael Catanzaro - 2.68.4-12 +- Fix race with waitpid() and child watcher sources +- Resolves: RHEL-14761 + * Wed Jul 19 2023 Michael Catanzaro - 2.68.4-11 - Really fix authentication failures when sd-bus clients connect to GDBus servers - Resolves: #2217771