Merge branch 'fix-data-race' into 'master'

Fix data race with members of struct DBusCounter

Closes #426

See merge request dbus/dbus!379

dbus/dbus-resources.c

@@ -275,7 +275,11 @@ _dbus_counter_adjust_unix_fd (DBusCounter *counter,
 long
 _dbus_counter_get_size_value (DBusCounter *counter)
 {
-  return counter->size_value;
+  long result;
+  _dbus_rmutex_lock (counter->mutex);
+  result = counter->size_value;
+  _dbus_rmutex_unlock (counter->mutex);
+  return result;
 }
 
 /**
@@ -287,7 +291,11 @@ _dbus_counter_get_size_value (DBusCounter *counter)
 long
 _dbus_counter_get_unix_fd_value (DBusCounter *counter)
 {
-  return counter->unix_fd_value;
+  long result;
+  _dbus_rmutex_lock (counter->mutex);
+  result = counter->unix_fd_value;
+  _dbus_rmutex_unlock (counter->mutex);
+  return result;
 }
 
 /**

dbus/dbus-resources.h

@@ -26,6 +26,7 @@
 #define DBUS_RESOURCES_H
 
 #include <dbus/dbus-macros.h>
+#include <dbus/dbus-macros-internal.h>
 #include <dbus/dbus-errors.h>
 #include <dbus/dbus-connection.h>
 
@@ -35,17 +36,22 @@ typedef struct DBusCounter DBusCounter;
 
 typedef void (* DBusCounterNotifyFunction) (DBusCounter *counter,
                                             void        *user_data);
-
+DBUS_EMBEDDED_TESTS_EXPORT
 DBusCounter* _dbus_counter_new       (void);
 DBusCounter* _dbus_counter_ref       (DBusCounter *counter);
+DBUS_EMBEDDED_TESTS_EXPORT
 void         _dbus_counter_unref     (DBusCounter *counter);
 
+DBUS_EMBEDDED_TESTS_EXPORT
 void         _dbus_counter_adjust_size       (DBusCounter *counter,
                                               long         delta);
+DBUS_EMBEDDED_TESTS_EXPORT
 void         _dbus_counter_adjust_unix_fd    (DBusCounter *counter,
                                               long         delta);
 void         _dbus_counter_notify            (DBusCounter *counter);
+DBUS_EMBEDDED_TESTS_EXPORT
 long         _dbus_counter_get_size_value    (DBusCounter *counter);
+DBUS_EMBEDDED_TESTS_EXPORT
 long         _dbus_counter_get_unix_fd_value (DBusCounter *counter);
 
 void _dbus_counter_set_notify    (DBusCounter               *counter,

test/CMakeLists.txt

@@ -163,6 +163,7 @@ if(DBUS_ENABLE_EMBEDDED_TESTS)
 
     if(NOT WIN32)
         add_test_executable(test-bus-system bus/system.c launch-helper-internal dbus-testutils)
+        add_test_executable(test-counter internals/counter.c dbus-testutils)
         if(ENABLE_TRADITIONAL_ACTIVATION)
             add_test_executable(test-bus-launch-helper-oom bus/launch-helper-oom.c launch-helper-internal dbus-testutils)
             add_helper_executable(dbus-daemon-launch-helper-for-tests bus/launch-helper-for-tests.c launch-helper-internal)

test/Makefile.am

@@ -97,6 +97,9 @@ uninstallable_test_programs += test-normal-activation
 endif
 
 if DBUS_UNIX
+test_counter_SOURCES = internals/counter.c
+test_counter_LDADD = libdbus-testutils.la
+
 if ENABLE_TRADITIONAL_ACTIVATION
 uninstallable_test_programs += test-bus-launch-helper-oom
 uninstallable_test_programs += test-bus-system
@@ -391,6 +394,14 @@ installable_manual_tests += \
 	$(NULL)
 endif DBUS_WITH_GLIB
 
+if DBUS_UNIX
+if DBUS_ENABLE_EMBEDDED_TESTS
+installable_tests += \
+	test-counter \
+	$(NULL)
+endif DBUS_ENABLE_EMBEDDED_TESTS
+endif DBUS_UNIX
+
 installable_test_meta = \
 	$(dist_installable_test_scripts:=.test) \
 	$(dist_installed_test_scripts:=.test) \

test/internals/counter.c

@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 2023 Ralf Habacker
+ * SPDX-License-Identifier: MIT
+ */
+
+#include <config.h>
+
+#include "dbus/dbus-resources.h"
+#include "dbus/dbus-test.h"
+#include "dbus/dbus-test-tap.h"
+#include "test/test-utils.h"
+
+#include <pthread.h>
+
+static dbus_bool_t verbose = FALSE;
+static DBusCounter *counter = NULL;
+
+static void* unix_fd_thread1 (void *arg _DBUS_GNUC_UNUSED)
+{
+  long i;
+  for (i=0; i < 10000; i++)
+    {
+      long j;
+      _dbus_counter_adjust_unix_fd (counter, 1);
+      j = _dbus_counter_get_unix_fd_value (counter);
+      if (verbose)
+        _dbus_test_diag ("write %ld\n", j);
+    }
+  return NULL;
+}
+
+static void* unix_fd_thread2 (void *arg _DBUS_GNUC_UNUSED)
+{
+  long j = 0;
+  do
+    {
+      j = _dbus_counter_get_unix_fd_value (counter);
+      if (verbose)
+        _dbus_test_diag ("read %ld\n", j);
+    } while (j < 10000);
+  return NULL;
+}
+
+static void* size_value_thread1 (void *arg _DBUS_GNUC_UNUSED)
+{
+  long i;
+  for (i=0; i < 10000; i++)
+    {
+      long j;
+      _dbus_counter_adjust_size (counter, 1);
+      j = _dbus_counter_get_size_value (counter);
+      if (verbose)
+        _dbus_test_diag ("write %ld\n", j);
+    }
+  return NULL;
+}
+
+static void* size_value_thread2 (void *arg _DBUS_GNUC_UNUSED)
+{
+  long j = 0;
+  do
+    {
+      j = _dbus_counter_get_size_value (counter);
+      if (verbose)
+        _dbus_test_diag("read %ld\n", j);
+    } while (j < 10000);
+  return NULL;
+}
+
+static dbus_bool_t
+_dbus_counter_unix_fd_test (const char *test_data_dir _DBUS_GNUC_UNUSED)
+{
+  pthread_t tid[2];
+  dbus_bool_t ret = TRUE;
+  counter = _dbus_counter_new ();
+
+  pthread_create (&(tid[0]), NULL, &unix_fd_thread1, NULL);
+  pthread_create (&(tid[1]), NULL, &unix_fd_thread2, NULL);
+
+  pthread_join (tid[0], NULL);
+  pthread_join (tid[1], NULL);
+
+  _dbus_counter_unref (counter);
+
+  return ret;
+}
+
+static dbus_bool_t
+_dbus_counter_size_value_test (const char *test_data_dir _DBUS_GNUC_UNUSED)
+{
+  pthread_t tid[2];
+  dbus_bool_t ret = TRUE;
+  counter = _dbus_counter_new ();
+
+  pthread_create (&(tid[0]), NULL, &size_value_thread1, NULL);
+  pthread_create (&(tid[1]), NULL, &size_value_thread2, NULL);
+
+  pthread_join (tid[0], NULL);
+  pthread_join (tid[1], NULL);
+
+  _dbus_counter_unref (counter);
+
+  return ret;
+}
+
+static const DBusTestCase test[] =
+{
+  { "unix_fd", _dbus_counter_unix_fd_test },
+  { "size", _dbus_counter_size_value_test },
+};
+
+
+int
+main (int    argc,
+      char **argv)
+{
+  return _dbus_test_main (argc, argv, sizeof(test) / sizeof (DBusTestCase), test,
+                          DBUS_TEST_FLAGS_CHECK_MEMORY_LEAKS,
+                          NULL, NULL);
+}

test/meson.build

@@ -298,6 +298,16 @@ if embedded_tests
         ]
     endif
 
+    if platform_unix
+        tests += [
+            {
+            'name': 'counter',
+            'srcs': [ 'internals/counter.c' ],
+            'link': [ libdbus_testutils, ],
+            },
+        ]
+    endif
+
     if use_traditional_activation and platform_unix
         tests += [
             {