From: Serge Hallyn Date: Thu, 2 Jan 2014 15:40:16 +0000 (-0600) Subject: Revert "Use pthread_atfork() to unlock mutexes after fork()" X-Git-Tag: lxc-1.0.0.beta2~97 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=64b1be2903078ef9e9ba3ffcbc30a4dc9bc5cc6c;p=thirdparty%2Flxc.git Revert "Use pthread_atfork() to unlock mutexes after fork()" This reverts commit 84e9e197933e601b66480da578b92630ebedfc72, because it breaks bionic builds. The patch is desirable so hopefully we can come up with a solution or alternate patch soon. --- diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am index f18d6e176..74b38e2db 100644 --- a/src/lxc/Makefile.am +++ b/src/lxc/Makefile.am @@ -135,10 +135,9 @@ AM_CFLAGS += -DHAVE_SECCOMP liblxc_so_SOURCES += seccomp.c endif -liblxc_so_CFLAGS = -fPIC -DPIC $(AM_CFLAGS) -pthread +liblxc_so_CFLAGS = -fPIC -DPIC $(AM_CFLAGS) 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 252e172a2..674961731 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -497,6 +497,7 @@ char *lxc_attach_getpwshell(uid_t uid) NULL }; + process_unlock(); // we're no longer sharing close(pipes[0]); /* we want to capture stdout */ @@ -789,6 +790,7 @@ 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 d6175c111..29e2ad57f 100644 --- a/src/lxc/bdev.c +++ b/src/lxc/bdev.c @@ -73,6 +73,7 @@ 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) @@ -197,6 +198,7 @@ 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); @@ -281,6 +283,7 @@ 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); @@ -572,6 +575,7 @@ 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); @@ -595,6 +599,7 @@ 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); } @@ -605,6 +610,7 @@ 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); } @@ -615,6 +621,7 @@ 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); } @@ -665,6 +672,7 @@ 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; @@ -709,6 +717,7 @@ 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) @@ -853,6 +862,7 @@ 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) @@ -912,6 +922,7 @@ 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) @@ -1045,6 +1056,7 @@ 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); } @@ -2080,6 +2092,7 @@ 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 e1cbd76f5..b62f7ba6e 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -601,6 +601,7 @@ 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) { @@ -838,6 +839,7 @@ 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); @@ -1234,6 +1236,7 @@ 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); @@ -2325,6 +2328,7 @@ 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 69d3d158a..b30f4c342 100644 --- a/src/lxc/lxclock.c +++ b/src/lxc/lxclock.c @@ -89,7 +89,7 @@ void unlock_mutex(pthread_mutex_t *l) int ret; if ((ret = pthread_mutex_unlock(l)) != 0) { - fprintf(stderr, "pthread_mutex_unlock returned:%d %s", ret, strerror(ret)); + fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret, strerror(ret)); dump_stacktrace(); exit(1); } @@ -317,21 +317,6 @@ 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) { @@ -343,12 +328,6 @@ 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 1fe117072..2cc4831af 100644 --- a/src/lxc/monitor.c +++ b/src/lxc/monitor.c @@ -299,6 +299,7 @@ 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);