From e1413111a79da9554be32ea1cabc3f88c0893d8c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 13 Nov 2018 19:01:54 +0100 Subject: [PATCH] core: minor cleanup of initializing nm_utils_get_testing() - add a commnt about thread-safety. - minor refactoring initializing the value in nm_utils_get_testing(). Instead of returning the flags we just set, go back to the begin and re-read the value (which must be initialized by now). No big difference, but feels a bit nicer to me. --- src/nm-core-utils.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 61d484f3ac..3d7047c3c9 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -52,6 +52,17 @@ #endif G_STATIC_ASSERT (sizeof (NMUtilsTestFlags) <= sizeof (int)); + +/* we read _nm_utils_testing without memory barrier. This is thread-safe, + * because the static variable is initialized to zero, and only reset + * once to a non-zero value (via g_atomic_int_compare_and_exchange()). + * + * Since there is only one integer that contains the data, there is no + * caching problem reading this (atomic int) variable without + * synchronization/memory-barrier. Contrary to a double-checked locking, + * where one needs a memory barrier to read the variable and ensure + * that also the related data is coherent in cache. Here there is no + * related data. */ static int _nm_utils_testing = 0; gboolean @@ -70,6 +81,7 @@ nm_utils_get_testing () { NMUtilsTestFlags flags; +again: flags = (NMUtilsTestFlags) _nm_utils_testing; if (flags != NM_UTILS_TEST_NONE) { /* Flags already initialized. Return them. */ @@ -82,12 +94,11 @@ nm_utils_get_testing () if (g_test_initialized ()) flags |= _NM_UTILS_TEST_GENERAL; - if (g_atomic_int_compare_and_exchange (&_nm_utils_testing, 0, (int) flags)) { - /* Done. We set it. */ - return flags & NM_UTILS_TEST_ALL; - } - /* It changed in the meantime (??). Re-read the value. */ - return ((NMUtilsTestFlags) _nm_utils_testing) & NM_UTILS_TEST_ALL; + g_atomic_int_compare_and_exchange (&_nm_utils_testing, 0, (int) flags); + + /* regardless of whether we won the race of initializing _nm_utils_testing, + * go back and read the value again. It must be non-zero by now. */ + goto again; } void