From ba28684ac2acd7dd97b1deeaf34259db023bad0e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Aug 2017 12:18:22 -0400 Subject: [PATCH] lib/deploy: Really close testing race condition I added `waitpid()`, but that didn't actually help because we were `daemon()`izing. Don't daemonize if we're testing so that we can `waitpid()`. Note I still haven't reproduced this race locally, but I'm pretty sure this will fix it. While here, actually check the return value from `waitpid()` just in case something goes wrong there. Closes: #1084 Approved by: jlebon --- src/libostree/ostree-sysroot-deploy.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 18624640..cb84a87f 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1020,10 +1020,14 @@ fsfreeze_thaw_cycle (OstreeSysroot *self, (void) close (glnx_steal_fd (&sock_parent)); /* Daemonize, and mask SIGINT/SIGTERM, so we're likely to survive e.g. * someone doing a `systemctl restart rpm-ostreed` or a Ctrl-C of - * `ostree admin upgrade`. + * `ostree admin upgrade`. We don't daemonize though if testing so + * that we can waitpid(). */ - if (daemon (0, debug_fifreeze ? 1 : 0) < 0) - err (1, "daemon"); + if (!debug_fifreeze) + { + if (daemon (0, 0) < 0) + err (1, "daemon"); + } int sigs[] = { SIGINT, SIGTERM }; for (guint i = 0; i < G_N_ELEMENTS (sigs); i++) { @@ -1086,7 +1090,10 @@ fsfreeze_thaw_cycle (OstreeSysroot *self, { int wstatus; /* Ensure the child has written its data */ - (void) TEMP_FAILURE_RETRY (waitpid (pid, &wstatus, 0)); + if (TEMP_FAILURE_RETRY (waitpid (pid, &wstatus, 0)) < 0) + return glnx_throw_errno_prefix (error, "waitpid(test-fifreeze)"); + if (!g_spawn_check_exit_status (wstatus, error)) + return glnx_prefix_error (error, "test-fifreeze: "); return glnx_throw (error, "aborting due to test-fifreeze"); } /* Do a freeze/thaw cycle; TODO add a FIFREEZETHAW ioctl */