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

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)))
 

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
 	 */

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);

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);

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);

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);