platform: make NMPNetns thread-safe

NMPNetns instances are immutable, hence they can be easily shared
between threads. All we need, is that the stack of namespaces is
thread-local.

Also note that NMPNetns uses almost no other API, except some bits from
"shared/nm-utils/" and nm-logging. These parts are already supposed to
be thread-safe.

The only complications is that when the thread exits, we need to
destroy the NMPNetns instances. That is especially important because
they hold file descriptors. This is accomplished using pthread's
thread-specific data. An alternative would be C11 threads' tss_create(),
but not all systems that we run against support that yet. This means,
we need to link with pthreads, but we already do that anyway.
Note that glib also requires pthreads. So, we don't get an additional
dependency here.

2 files changed, 124 insertions, 22 deletions

diff --git a/src/platform/nmp-netns.c b/src/platform/nmp-netns.c
index f1092fe93b..0c84bb4019 100644
--- a/src/platform/nmp-netns.c
+++ b/src/platform/nmp-netns.c
@@ -19,6 +19,7 @@
  */
 
 #include "nm-default.h"
+
 #include "nmp-netns.h"
 
 #include <fcntl.h>
@@ -26,8 +27,19 @@
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <pthread.h>
+
+/*****************************************************************************/
 
-#include "NetworkManagerUtils.h"
+/* NOTE: NMPNetns and all code used here must be thread-safe! */
+
+/* we may not call logging functions from the main-thread alone. Hence, we
+ * require locking from nm-logging. Indicate that by setting NM_THREAD_SAFE_ON_MAIN_THREAD
+ * to zero. */
+#undef NM_THREAD_SAFE_ON_MAIN_THREAD
+#define NM_THREAD_SAFE_ON_MAIN_THREAD 0
+
+/*****************************************************************************/
 
 #define PROC_SELF_NS_MNT "/proc/self/ns/mnt"
 #define PROC_SELF_NS_NET "/proc/self/ns/net"
@@ -121,42 +133,67 @@ static NMPNetns *_netns_new (GError **error);
 
 /*****************************************************************************/
 
-static GArray *netns_stack = NULL;
+static _nm_thread_local GArray *netns_stack = NULL;
 
 static void
-_stack_ensure_init_impl (void)
+_netns_stack_clear_cb (gpointer data)
 {
-	NMPNetns *netns;
-	GError *error = NULL;
+	NetnsInfo *info = data;
 
-	nm_assert (!netns_stack);
+	nm_assert (NMP_IS_NETNS (info->netns));
+	g_object_unref (info->netns);
+}
 
-	netns_stack = g_array_new (FALSE, FALSE, sizeof (NetnsInfo));
+static GArray *
+_netns_stack_get_impl (void)
+{
+	gs_unref_object NMPNetns *netns = NULL;
+	gs_free_error GError *error = NULL;
+	pthread_key_t key;
+	GArray *s;
+
+	s = g_array_new (FALSE, FALSE, sizeof (NetnsInfo));
+	g_array_set_clear_func (s, _netns_stack_clear_cb);
+	netns_stack = s;
 
 	/* at the bottom of the stack we must try to create a netns instance
 	 * that we never pop. It's the base to which we need to return. */
-
 	netns = _netns_new (&error);
-
 	if (!netns) {
-		/* don't know how to recover from this error. Netns are not supported. */
 		_LOGE (NULL, "failed to create initial netns: %s", error->message);
-		g_clear_error (&error);
-		return;
+		return s;
 	}
 
+	/* we leak this instance inside the stack. */
 	_stack_push (netns, _CLONE_NS_ALL);
 
-	/* we leak this instance inside netns_stack. It cannot be popped. */
-	g_object_unref (netns);
+	/* finally, register a destructor function to cleanup the array. If we fail
+	 * to do so, we will leak NMPNetns instances (and their file descriptor) when the
+	 * thread exits. */
+	if (pthread_key_create (&key, (void (*) (void *)) g_array_unref) != 0)
+		_LOGE (NULL, "failure to initialize thread-local storage");
+	else if (pthread_setspecific (key, s) != 0)
+		_LOGE (NULL, "failure to set thread-local storage");
+
+	return s;
 }
+
+#define _netns_stack_get() \
+	({ \
+		GArray *_s = netns_stack; \
+		\
+		if (G_UNLIKELY (!_s)) \
+			_s = _netns_stack_get_impl (); \
+		_s; \
+	})
+
 #define _stack_ensure_init() \
 	G_STMT_START { \
-		if (G_UNLIKELY (!netns_stack)) { \
-			_stack_ensure_init_impl (); \
-		} \
+		(void) _netns_stack_get (); \
 	} G_STMT_END
 
+/*****************************************************************************/
+
 static NMPNetns *
 _stack_current_netns (int ns_types)
 {
@@ -244,9 +281,11 @@ _stack_push (NMPNetns *netns, int ns_types)
 	g_array_set_size (netns_stack, netns_stack->len + 1);
 
 	info = &g_array_index (netns_stack, NetnsInfo, (netns_stack->len - 1));
-	info->netns = g_object_ref (netns);
-	info->ns_types = ns_types;
-	info->count = 1;
+	*info = (NetnsInfo) {
+		.netns    = g_object_ref (netns),
+		.ns_types = ns_types,
+		.count    = 1,
+	};
 }
 
 static void
@@ -262,8 +301,6 @@ _stack_pop (void)
 	nm_assert (NMP_IS_NETNS (info->netns));
 	nm_assert (info->count == 1);
 
-	g_object_unref (info->netns);
-
 	g_array_set_size (netns_stack, netns_stack->len - 1);
 }
 
diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c
index bfd330580a..21f1a80dbb 100644
--- a/src/platform/tests/test-link.c
+++ b/src/platform/tests/test-link.c
@@ -2983,6 +2983,69 @@ test_sysctl_netns_switch (void)
 
 /*****************************************************************************/
 
+static gpointer
+_test_netns_mt_thread (gpointer data)
+{
+	NMPNetns *netns1 = data;
+	gs_unref_object NMPNetns *netns2 = NULL;
+	NMPNetns *netns_bottom;
+	NMPNetns *initial;
+
+	netns_bottom = nmp_netns_get_initial ();
+	g_assert (netns_bottom);
+
+	/* I don't know why, but we need to create a new netns here at least once.
+	 * Otherwise, setns(, CLONE_NEWNS) below fails with EINVAL (???).
+	 *
+	 * Something is not right here, but what?  */
+	netns2 = nmp_netns_new ();
+	nmp_netns_pop (netns2);
+	g_clear_object (&netns2);
+
+	nmp_netns_push (netns1);
+	nmp_netns_push_type (netns_bottom, CLONE_NEWNET);
+	nmp_netns_push_type (netns_bottom, CLONE_NEWNS);
+	nmp_netns_push_type (netns1, CLONE_NEWNS);
+	nmp_netns_pop (netns1);
+	nmp_netns_pop (netns_bottom);
+	nmp_netns_pop (netns_bottom);
+	nmp_netns_pop (netns1);
+
+	initial = nmp_netns_get_initial ();
+	g_assert (NMP_IS_NETNS (initial));
+	return g_object_ref (initial);
+}
+
+static void
+test_netns_mt (void)
+{
+	gs_unref_object NMPNetns *netns1 = NULL;
+	NMPNetns *initial_from_other_thread;
+	GThread *th;
+
+	if (_test_netns_check_skip ())
+		return;
+
+	netns1 = nmp_netns_new ();
+	g_assert (NMP_NETNS (netns1));
+	nmp_netns_pop (netns1);
+
+	th = g_thread_new ("nm-test-netns-mt", _test_netns_mt_thread, netns1);
+	initial_from_other_thread = g_thread_join (th);
+	g_assert (NMP_IS_NETNS (initial_from_other_thread));
+
+	if (nmtst_get_rand_bool ()) {
+		nmp_netns_push (initial_from_other_thread);
+		nmp_netns_pop (initial_from_other_thread);
+	}
+
+	g_object_add_weak_pointer (G_OBJECT (initial_from_other_thread), (gpointer *) &initial_from_other_thread);
+	g_object_unref (initial_from_other_thread);
+	g_assert (initial_from_other_thread == NULL);
+}
+
+/*****************************************************************************/
+
 static void
 ethtool_features_dump (const NMEthtoolFeatureStates *features)
 {
@@ -3137,6 +3200,8 @@ _nmtstp_setup_tests (void)
 		g_test_add_vtable ("/general/netns/push", 0, NULL, _test_netns_setup, test_netns_push, _test_netns_teardown);
 		g_test_add_vtable ("/general/netns/bind-to-path", 0, NULL, _test_netns_setup, test_netns_bind_to_path, _test_netns_teardown);
 
+		g_test_add_func ("/general/netns/mt", test_netns_mt);
+
 		g_test_add_func ("/general/sysctl/rename", test_sysctl_rename);
 		g_test_add_func ("/general/sysctl/netns-switch", test_sysctl_netns_switch);