Pending patches for MT-fork stuff

Submitted by Rich Felker on Sept. 28, 2020, 11:26 p.m.

Details

Message ID 20200928232614.GA17637@brightrain.aerifal.cx
State New
Series "Pending patches for MT-fork stuff"
Headers show

Commit Message

Rich Felker Sept. 28, 2020, 11:26 p.m.
In investigating the MT-fork deadlock stuff and working on a patch
that makes it attempt to trap rather than deadlocking, I found a
problem with interaction of fork and aio. A fix for that, along with
the patch to trap, and a minimal testcase for the aio bug, are
attached.

There's also a problematic interaction of abort with fork that was
just found in glibc that also exists in musl, that I still need to
fix. I'll follow up later with a proposed solution for that.

Rich
From 34904d830a9fd1f6fc47218f38c111698303d2fe Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Mon, 28 Sep 2020 18:38:27 -0400
Subject: [PATCH 1/3] fix fork of processes with active async io contexts

previously, if a file descriptor had aio operations pending in the
parent before fork, attempting to close it in the child would attempt
to cancel a thread belonging to the parent. this could deadlock, fail,
or crash the whole process of the cancellation signal handler was not
yet installed in the parent. in addition, further use of aio from the
child could malfunction or deadlock.

POSIX specifies that async io operations are not inherited by the
child on fork, so clear the entire aio fd map in the child, and take
the aio map lock (with signals blocked) across the fork so that the
lock is kept in a consistent state.
---
 src/aio/aio.c               | 14 ++++++++++++++
 src/internal/pthread_impl.h |  2 ++
 src/process/fork.c          |  3 +++
 3 files changed, 19 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/aio/aio.c b/src/aio/aio.c
index 6d34fa86..f59679c3 100644
--- a/src/aio/aio.c
+++ b/src/aio/aio.c
@@ -392,6 +392,20 @@  int __aio_close(int fd)
 	return fd;
 }
 
+void __aio_atfork(int who)
+{
+	if (who<0) {
+		pthread_rwlock_rdlock(&maplock);
+		return;
+	}
+	if (who>0 && map) for (int a=0; a<(-1U/2+1)>>24; a++)
+		if (map[a]) for (int b=0; b<256; b++)
+			if (map[a][b]) for (int c=0; c<256; c++)
+				if (map[a][b][c]) for (int d=0; d<256; d++)
+					map[a][b][c][d] = 0;
+	pthread_rwlock_unlock(&maplock);
+}
+
 weak_alias(aio_cancel, aio_cancel64);
 weak_alias(aio_error, aio_error64);
 weak_alias(aio_fsync, aio_fsync64);
diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
index 4d709bbc..358ad1ce 100644
--- a/src/internal/pthread_impl.h
+++ b/src/internal/pthread_impl.h
@@ -162,6 +162,8 @@  extern hidden void *__pthread_tsd_main[];
 extern hidden volatile int __aio_fut;
 extern hidden volatile int __eintr_valid_flag;
 
+extern hidden void __aio_atfork(int);
+
 hidden int __clone(int (*)(void *), void *, int, void *, ...);
 hidden int __set_thread_area(void *);
 hidden int __libc_sigaction(int, const struct sigaction *, struct sigaction *);
diff --git a/src/process/fork.c b/src/process/fork.c
index 7e984ff8..dbaa9402 100644
--- a/src/process/fork.c
+++ b/src/process/fork.c
@@ -10,6 +10,7 @@  static void dummy(int x)
 }
 
 weak_alias(dummy, __fork_handler);
+weak_alias(dummy, __aio_atfork);
 
 pid_t fork(void)
 {
@@ -17,6 +18,7 @@  pid_t fork(void)
 	sigset_t set;
 	__fork_handler(-1);
 	__block_all_sigs(&set);
+	__aio_atfork(-1);
 #ifdef SYS_fork
 	ret = __syscall(SYS_fork);
 #else
@@ -32,6 +34,7 @@  pid_t fork(void)
 		libc.threads_minus_1 = 0;
 		if (libc.need_locks) libc.need_locks = -1;
 	}
+	__aio_atfork(!ret);
 	__restore_sigs(&set);
 	__fork_handler(!ret);
 	return __syscall_ret(ret);