From: Andrey Mazo Date: Mon, 30 Dec 2013 11:06:25 +0000 (+0400) Subject: Use pthread_atfork() to unlock mutexes after fork() X-Git-Tag: lxc-1.0.0.beta2~99 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=84e9e197933e601b66480da578b92630ebedfc72;p=thirdparty%2Flxc.git Use pthread_atfork() to unlock mutexes after fork() Signed-off-by: Andrey Mazo Signed-off-by: Serge Hallyn --- diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am index 74b38e2db..f18d6e176 100644 --- 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 674961731..252e172a2 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -497,7 +497,6 @@ 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 29e2ad57f..d6175c111 100644 --- 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); @@ -575,7 +572,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); @@ -599,7 +595,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); } @@ -610,7 +605,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); } @@ -621,7 +615,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); } @@ -672,7 +665,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; @@ -717,7 +709,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) @@ -862,7 +853,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) @@ -922,7 +912,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) @@ -1056,7 +1045,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); } @@ -2092,7 +2080,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 b62f7ba6e..e1cbd76f5 100644 --- 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 b30f4c342..69d3d158a 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_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 2cc4831af..1fe117072 100644 --- 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);