Revert "Revert "Use pthread_atfork() to unlock mutexes after fork()""

This reverts commit 64b1be2903078ef9e9ba3ffcbc30a4dc9bc5cc6c.

Reverting in preparation for another implementation which is
bionic-compatible.

Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>

diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
index c54120ab1af70a9e4bbf1144ca0f5bcb20df55cb..9ad091cf6ec5c83a85bb6777b570f8cd2a66bfdd 100644 (file)
--- a/src/lxc/Makefile.am
+++ b/src/lxc/Makefile.am
@@ -135,9 +135,10 @@ AM_CFLAGS += -DHAVE_SECCOMP
 liblxc_so_SOURCES += seccomp.c
 endif
 
-liblxc_so_CFLAGS = -fPIC -DPIC $(AM_CFLAGS)
+liblxc_so_CFLAGS = -fPIC -DPIC $(AM_CFLAGS) -pthread
 
 liblxc_so_LDFLAGS = \
+       -pthread \
        -shared \
        -Wl,-soname,liblxc.so.$(firstword $(subst ., ,$(VERSION)))
 

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index b7551d919afdc8c26f466c84412817a9f411b5c7..422f24cac74f75b0e9a5c7cdf15cffe6b99d7d5e 100644 (file)
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -497,7 +497,6 @@ static char *lxc_attach_getpwshell(uid_t uid)
                        NULL
                };
 
-               process_unlock(); // we're no longer sharing
                close(pipes[0]);
 
                /* we want to capture stdout */
@@ -790,7 +789,6 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
                return -1;
        }
 
-       process_unlock(); // we're no longer sharing
        /* first subprocess begins here, we close the socket that is for the
         * initial thread
         */

diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
index f5d750d6ad99d0b0c8f0d74676cc60c52e5da060..2e4281dc48d0359edacce7f8a2ba7e8e59175d94 100644 (file)
--- a/src/lxc/bdev.c
+++ b/src/lxc/bdev.c
@@ -73,7 +73,6 @@ static int do_rsync(const char *src, const char *dest)
        if (pid > 0)
                return wait_for_pid(pid);
 
-       process_unlock(); // we're no longer sharing
        l = strlen(src) + 2;
        s = malloc(l);
        if (!s)
@@ -198,7 +197,6 @@ static int do_mkfs(const char *path, const char *fstype)
        if (pid > 0)
                return wait_for_pid(pid);
 
-       process_unlock(); // we're no longer sharing
        // If the file is not a block device, we don't want mkfs to ask
        // us about whether to proceed.
        close(0);
@@ -283,7 +281,6 @@ static int detect_fs(struct bdev *bdev, char *type, int len)
                return ret;
        }
 
-       process_unlock(); // we're no longer sharing
        if (unshare(CLONE_NEWNS) < 0)
                exit(1);
 
@@ -576,7 +573,6 @@ static int zfs_clone(const char *opath, const char *npath, const char *oname,
                if (!pid) {
                        char dev[MAXPATHLEN];
 
-                       process_unlock(); // we're no longer sharing
                        ret = snprintf(dev, MAXPATHLEN, "%s/%s", zfsroot, nname);
                        if (ret < 0  || ret >= MAXPATHLEN)
                                exit(1);
@@ -600,7 +596,6 @@ static int zfs_clone(const char *opath, const char *npath, const char *oname,
                if ((pid = fork()) < 0)
                        return -1;
                if (!pid) {
-                       process_unlock(); // we're no longer sharing
                        execlp("zfs", "zfs", "destroy", path1, NULL);
                        exit(1);
                }
@@ -611,7 +606,6 @@ static int zfs_clone(const char *opath, const char *npath, const char *oname,
                if ((pid = fork()) < 0)
                        return -1;
                if (!pid) {
-                       process_unlock(); // we're no longer sharing
                        execlp("zfs", "zfs", "snapshot", path1, NULL);
                        exit(1);
                }
@@ -622,7 +616,6 @@ static int zfs_clone(const char *opath, const char *npath, const char *oname,
                if ((pid = fork()) < 0)
                        return -1;
                if (!pid) {
-                       process_unlock(); // we're no longer sharing
                        execlp("zfs", "zfs", "clone", option, path1, path2, NULL);
                        exit(1);
                }
@@ -673,7 +666,6 @@ static int zfs_destroy(struct bdev *orig)
        if (pid)
                return wait_for_pid(pid);
 
-       process_unlock(); // we're no longer sharing
        if (!zfs_list_entry(orig->src, output, MAXPATHLEN)) {
                ERROR("Error: zfs entry for %s not found", orig->src);
                return -1;
@@ -718,7 +710,6 @@ static int zfs_create(struct bdev *bdev, const char *dest, const char *n,
        if (pid)
                return wait_for_pid(pid);
 
-       process_unlock(); // we're no longer sharing
        char dev[MAXPATHLEN];
        ret = snprintf(dev, MAXPATHLEN, "%s/%s", zfsroot, n);
        if (ret < 0  || ret >= MAXPATHLEN)
@@ -864,7 +855,6 @@ static int do_lvm_create(const char *path, unsigned long size, const char *thinp
        if (pid > 0)
                return wait_for_pid(pid);
 
-       process_unlock(); // we're no longer sharing
        // lvcreate default size is in M, not bytes.
        ret = snprintf(sz, 24, "%lu", size/1000000);
        if (ret < 0 || ret >= 24)
@@ -924,7 +914,6 @@ static int lvm_snapshot(const char *orig, const char *path, unsigned long size)
        if (pid > 0)
                return wait_for_pid(pid);
 
-       process_unlock(); // we're no longer sharing
        // lvcreate default size is in M, not bytes.
        ret = snprintf(sz, 24, "%lu", size/1000000);
        if (ret < 0 || ret >= 24)
@@ -1058,7 +1047,6 @@ static int lvm_destroy(struct bdev *orig)
        if ((pid = fork()) < 0)
                return -1;
        if (!pid) {
-               process_unlock(); // we're no longer sharing
                execlp("lvremove", "lvremove", "-f", orig->src, NULL);
                exit(1);
        }
@@ -2109,7 +2097,6 @@ struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
                return new;
        }
 
-       process_unlock(); // we're no longer sharing
        if (unshare(CLONE_NEWNS) < 0) {
                SYSERROR("unshare CLONE_NEWNS");
                exit(1);

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 75f6ce363de05bbff150491d5f3dfc60dafd6de3..132f1a3557dd0b5495be186d3147d8095b36a97e 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -601,7 +601,6 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
                if (pid != 0)
                        return wait_on_daemonized_start(c, pid);
 
-               process_unlock(); // we're no longer sharing
                /* second fork to be reparented by init */
                pid = fork();
                if (pid < 0) {
@@ -839,7 +838,6 @@ static bool create_run_template(struct lxc_container *c, char *tpath, bool quiet
                char **newargv;
                struct lxc_conf *conf = c->lxc_conf;
 
-               process_unlock(); // we're no longer sharing
                if (quiet) {
                        close(0);
                        close(1);
@@ -1236,7 +1234,6 @@ static bool lxcapi_create(struct lxc_container *c, const char *t,
        if (pid == 0) { // child
                struct bdev *bdev = NULL;
 
-               process_unlock(); // we're no longer sharing
                if (!(bdev = do_bdev_create(c, bdevtype, specs))) {
                        ERROR("Error creating backing store type %s for %s",
                                bdevtype ? bdevtype : "(none)", c->name);
@@ -2328,7 +2325,6 @@ static int clone_update_rootfs(struct lxc_container *c0,
        if (pid > 0)
                return wait_for_pid(pid);
 
-       process_unlock(); // we're no longer sharing
        bdev = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
        if (!bdev)
                exit(1);

diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
index 74b169a62afe0fb676f6795cff08191949c5dd0d..b0420bbdfd46178cb81d1eb5f6128e82f6e7168c 100644 (file)
--- a/src/lxc/lxclock.c
+++ b/src/lxc/lxclock.c
@@ -89,7 +89,7 @@ static void unlock_mutex(pthread_mutex_t *l)
        int ret;
 
        if ((ret = pthread_mutex_unlock(l)) != 0) {
-               fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret, strerror(ret));
+               fprintf(stderr, "pthread_mutex_unlock returned:%d %s", ret, strerror(ret));
                dump_stacktrace();
                exit(1);
        }
@@ -317,6 +317,21 @@ void process_unlock(void)
        unlock_mutex(&thread_mutex);
 }
 
+/* One thread can do fork() while another one is holding a mutex.
+ * There is only one thread in child just after the fork(), so noone will ever release that mutex.
+ * We setup a "child" fork handler to unlock the mutex just after the fork().
+ * For several mutex types, unlocking an unlocked mutex can lead to undefined behavior.
+ * One way to deal with it is to setup "prepare" fork handler
+ * to lock the mutex before fork() and both "parent" and "child" fork handlers
+ * to unlock the mutex.
+ * This forbids doing fork() while explicitly holding the lock.
+ */
+__attribute__((constructor))
+static void process_lock_setup_atfork(void)
+{
+       pthread_atfork(process_lock, process_unlock, process_unlock);
+}
+
 /* Protects static const values inside the lxc_global_config_value funtion */
 void static_lock(void)
 {
@@ -328,6 +343,12 @@ void static_unlock(void)
        unlock_mutex(&static_mutex);
 }
 
+__attribute__((constructor))
+static void static_lock_setup_atfork(void)
+{
+       pthread_atfork(static_lock, static_unlock, static_unlock);
+}
+
 int container_mem_lock(struct lxc_container *c)
 {
        return lxclock(c->privlock, 0);

diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index 2cc4831afd7c472b46a17460de570278475fcbcd..1fe117072b28e4e9ccf2a77182942708b5f154e3 100644 (file)
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -299,7 +299,6 @@ int lxc_monitord_spawn(const char *lxcpath)
                return 0;
        }
 
-       process_unlock(); // we're no longer sharing
        if (pipe(pipefd) < 0) {
                SYSERROR("failed to create pipe");
                exit(EXIT_FAILURE);