Locking data structures

If a data structure is associated with a lock, having a public
and a private part, there are now useful macros for these data
structures.

configure.ac

@@ -146,6 +146,9 @@ CFLAGS="$CFLAGS -fno-strict-aliasing -fno-strict-overflow"
 if test "$bird_cflags_default" = yes ; then
   BIRD_CHECK_GCC_OPTION([bird_cv_c_option_wno_pointer_sign], [-Wno-pointer-sign], [-Wall])
   BIRD_CHECK_GCC_OPTION([bird_cv_c_option_wno_missing_init], [-Wno-missing-field-initializers], [-Wall -Wextra])
+  BIRD_CHECK_GCC_OPTION([bird_cv_c_option_fms_extensions], [-fms-extensions], [-Wall -Wextra])
+  BIRD_CHECK_GCC_OPTION([bird_cv_c_option_wno_microsoft_anon_tag], [-Wno-microsoft-anon-tag], [-Wall -Wextra])
+
 
   if test "$enable_debug" = no; then
     BIRD_CHECK_LTO
@@ -161,6 +164,8 @@ if test "$bird_cflags_default" = yes ; then
   CFLAGS="$CFLAGS -Wall -Wextra -Wstrict-prototypes -Wno-parentheses"
   BIRD_ADD_GCC_OPTION([bird_cv_c_option_wno_pointer_sign], [-Wno-pointer-sign])
   BIRD_ADD_GCC_OPTION([bird_cv_c_option_wno_missing_init], [-Wno-missing-field-initializers])
+  BIRD_ADD_GCC_OPTION([bird_cv_c_option_fms_extensions], [-fms-extensions])
+  BIRD_ADD_GCC_OPTION([bird_cv_c_option_wno_microsoft_anon_tag], [-Wno-microsoft-anon-tag])
 fi
 
 

lib/Makefile

@@ -2,6 +2,6 @@ src := a-path.c a-set.c bitmap.c bitops.c blake2s.c blake2b.c checksum.c event.c
 obj := $(src-o-files)
 $(all-daemon)
 
-tests_src := a-set_test.c a-path_test.c attribute_cleanup_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c tlists_test.c type_test.c
+tests_src := a-set_test.c a-path_test.c attribute_cleanup_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c locking_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c tlists_test.c type_test.c
 tests_targets := $(tests_targets) $(tests-target-files)
 tests_objs := $(tests_objs) $(src-o-files)

lib/locking.h

@@ -72,4 +72,205 @@ extern DOMAIN(the_bird) the_bird_domain;
 
 #define ASSERT_THE_BIRD_LOCKED	({ if (!the_bird_locked()) bug("The BIRD lock must be locked here: %s:%d", __FILE__, __LINE__); })
 
+/**
+ *  Objects bound with domains
+ *
+ *  First, we need some object to have its locked and unlocked part.
+ *  This is accomplished typically by the following pattern:
+ *
+ *    struct foo_public {
+ *      ...			// Public fields
+ *      DOMAIN(bar) lock;	// The assigned domain
+ *    };
+ *
+ *    struct foo_private {
+ *      struct foo_public;	// Importing public fields
+ *      struct foo_private **locked_at;	// Auxiliary field for locking routines
+ *      ...			// Private fields
+ *    };
+ *
+ *    typedef union foo {
+ *      struct foo_public;
+ *      struct foo_private priv;
+ *    } foo;
+ *
+ *  All persistently stored object pointers MUST point to the public parts.
+ *  If accessing the locked object from embedded objects, great care must
+ *  be applied to always SKIP_BACK to the public object version, not the
+ *  private one.
+ *
+ *  To access the private object parts, either the private object pointer
+ *  is explicitly given to us, therefore assuming somewhere else the domain
+ *  has been locked, or we have to lock the domain ourselves. To do that,
+ *  there are some handy macros.
+ */
+
+#define LOBJ_LOCK_SIMPLE(_obj, _level) \
+  ({ LOCK_DOMAIN(_level, (_obj)->lock); &(_obj)->priv; })
+
+#define LOBJ_UNLOCK_SIMPLE(_obj, _level) \
+  UNLOCK_DOMAIN(_level, (_obj)->lock)
+
+/*
+ *  These macros can be used to define specific macros for given class.
+ *
+ *  #define FOO_LOCK_SIMPLE(foo)	LOBJ_LOCK_SIMPLE(foo, bar)
+ *  #define FOO_UNLOCK_SIMPLE(foo)	LOBJ_UNLOCK_SIMPLE(foo, bar)
+ *
+ *  Then these can be used like this:
+ *
+ *  void foo_frobnicate(foo *f)
+ *  {
+ *    // Unlocked context
+ *    ...
+ *    struct foo_private *fp = FOO_LOCK_SIMPLE(f);
+ *    // Locked context
+ *    ...
+ *    FOO_UNLOCK_SIMPLE(f);
+ *    // Unlocked context
+ *    ...
+ *  }
+ *
+ *  These simple calls have two major drawbacks. First, if you return
+ *  from locked context, you don't unlock, which may lock you dead.
+ *  And second, the foo_private pointer is still syntactically valid
+ *  even after unlocking.
+ *
+ *  To fight this, we need more magic and the switch should stay in that
+ *  position.
+ *
+ *  First, we need an auxiliary _function_ for unlocking. This function
+ *  is intended to be called in a local variable cleanup context.
+ */
+
+#define LOBJ_UNLOCK_CLEANUP_NAME(_stem) _lobj__##_stem##_unlock_cleanup
+
+#define LOBJ_UNLOCK_CLEANUP(_stem, _level) \
+  static inline void LOBJ_UNLOCK_CLEANUP_NAME(_stem)(struct _stem##_private **obj) { \
+    if (!*obj) return; \
+    ASSERT_DIE(LOBJ_IS_LOCKED((*obj), _level)); \
+    ASSERT_DIE((*obj)->locked_at == obj); \
+    (*obj)->locked_at = NULL; \
+    UNLOCK_DOMAIN(_level, (*obj)->lock); \
+  }
+
+#define LOBJ_LOCK(_obj, _pobj, _stem, _level) \
+  CLEANUP(LOBJ_UNLOCK_CLEANUP_NAME(_stem)) struct _stem##_private *_pobj = LOBJ_LOCK_SIMPLE(_obj, _level); _pobj->locked_at = &_pobj;
+
+/*
+ *  And now the usage of these macros. You first need to declare the auxiliary
+ *  cleanup function.
+ *
+ *  LOBJ_UNLOCK_CLEANUP(foo, bar);
+ *
+ *  And then declare the lock-local macro:
+ *
+ *  #define FOO_LOCK(foo, fpp)	LOBJ_LOCK(foo, fpp, foo, bar)
+ *
+ *  This construction then allows you to lock much more safely:
+ *
+ *  void foo_frobnicate_safer(foo *f)
+ *  {
+ *    // Unlocked context
+ *    ...
+ *    do {
+ *      FOO_LOCK(foo, fpp);
+ *	// Locked context, fpp is valid here
+ *
+ *	if (something) return;	// This implicitly unlocks
+ *	if (whatever) break;	// This unlocks too
+ *
+ *      // Finishing context with no unlock at all
+ *    } while (0);
+ *
+ *    // Here is fpp invalid and the object is back unlocked.
+ *    ...
+ *  }
+ *
+ *  There is no explicit unlock statement. To unlock, simply leave the block
+ *  with locked context.
+ *
+ *  This may be made even nicer to use by employing a for-cycle.
+ */
+
+#define LOBJ_LOCKED(_obj, _pobj, _stem, _level) \
+  for (CLEANUP(LOBJ_UNLOCK_CLEANUP_NAME(_stem)) struct _stem##_private *_pobj = LOBJ_LOCK_SIMPLE(_obj, _level); \
+      _pobj ? (_pobj->locked_at = &_pobj) : NULL; \
+      LOBJ_UNLOCK_CLEANUP_NAME(_stem)(&_pobj), _pobj = NULL)
+
+/*
+ *  This for-cycle employs heavy magic to hide as much of the boilerplate
+ *  from the user as possibly needed. Here is how it works.
+ *
+ *  First, the for-1 clause is executed, setting up _pobj, to the private
+ *  object pointer. It has a cleanup hook set.
+ *
+ *  Then, the for-2 clause is checked. As _pobj is non-NULL, _pobj->locked_at
+ *  is initialized to the _pobj address to ensure that the cleanup hook unlocks
+ *  the right object.
+ *
+ *  Now the user block is executed. If it ends by break or return, the cleanup
+ *  hook fires for _pobj, triggering object unlock.
+ *
+ *  If the user block executed completely, the for-3 clause is run, executing
+ *  the cleanup hook directly and then deactivating it by setting _pobj to NULL.
+ *
+ *  Finally, the for-2 clause is checked again but now with _pobj being NULL,
+ *  causing the loop to end. As the object has already been unlocked, nothing
+ *  happens after leaving the context.
+ *
+ *  #define FOO_LOCKED(foo, fpp)	LOBJ_LOCKED(foo, fpp, foo, bar)
+ *
+ *  Then the previous code can be modified like this:
+ *
+ *  void foo_frobnicate_safer(foo *f)
+ *  {
+ *    // Unlocked context
+ *    ...
+ *    FOO_LOCKED(foo, fpp)
+ *    {
+ *	// Locked context, fpp is valid here
+ *
+ *	if (something) return;	// This implicitly unlocks
+ *	if (whatever) break;	// This unlocks too
+ *
+ *      // Finishing context with no unlock at all
+ *    }
+ *
+ *    // Unlocked context
+ *    ...
+ *
+ *    // Locking once again without an explicit block
+ *    FOO_LOCKED(foo, fpp)
+ *	do_something(fpp);
+ *
+ *    // Here is fpp invalid and the object is back unlocked.
+ *    ...
+ *  }
+ *
+ *
+ *  For many reasons, a lock-check macro is handy.
+ *
+ *  #define FOO_IS_LOCKED(foo)	LOBJ_IS_LOCKED(foo, bar)
+ */
+
+#define LOBJ_IS_LOCKED(_obj, _level)	DOMAIN_IS_LOCKED(_level, (_obj)->lock)
+
+/*
+ *  An example implementation is available in lib/locking_test.c
+ */
+
+
+/*
+ *  Please don't use this macro unless you at least try to prove that
+ *  it's completely safe. It's a can of worms.
+ *
+ *  NEVER RETURN OR BREAK FROM THIS MACRO, it will crash.
+ */
+
+#define LOBJ_UNLOCKED_TEMPORARILY(_obj, _pobj, _stem, _level) \
+  for (union _stem *_obj = SKIP_BACK(union _stem, priv, _pobj), **_lataux = (union _stem **) _pobj->locked_at; \
+      _obj ? (_pobj->locked_at = NULL, LOBJ_UNLOCK_SIMPLE(_obj, _level), _obj) : NULL; \
+      LOBJ_LOCK_SIMPLE(_obj, _level), _pobj->locked_at = (struct _stem##_private **) _lataux, _obj = NULL)
+
 #endif

lib/locking_test.c

@@ -0,0 +1,92 @@
+#include "test/birdtest.h"
+#include "test/bt-utils.h"
+
+#include "lib/locking.h"
+#include <stdatomic.h>
+#include <pthread.h>
+
+DEFINE_DOMAIN(proto);
+
+struct foo_public {
+  const char *name;
+  _Atomic uint counter;
+  DOMAIN(proto) lock;
+};
+
+struct foo_private {
+  struct foo_public;
+  struct foo_private **locked_at;
+  uint private_counter;
+};
+
+typedef union foo {
+  struct foo_public;
+  struct foo_private priv;
+} foo;
+
+LOBJ_UNLOCK_CLEANUP(foo, proto);
+#define FOO_LOCK(_foo, _fpp)	LOBJ_LOCK(_foo, _fpp, foo, proto)
+#define FOO_LOCKED(_foo, _fpp)	LOBJ_LOCKED(_foo, _fpp, foo, proto)
+#define FOO_IS_LOCKED(_foo)	LOBJ_IS_LOCKED(_foo, proto)
+
+static uint
+inc_public(foo *f)
+{
+  return atomic_fetch_add_explicit(&f->counter, 1, memory_order_relaxed) + 1;
+}
+
+static uint
+inc_private(foo *f)
+{
+  FOO_LOCKED(f, fp) return ++fp->private_counter;
+  bug("Returning always");
+}
+
+#define BLOCKCOUNT  4096
+#define THREADS	    16
+#define REPEATS	    128
+
+static void *
+thread_run(void *_foo)
+{
+  foo *f = _foo;
+
+  for (int i=0; i<REPEATS; i++)
+    if (i % 2)
+      for (int j=0; j<BLOCKCOUNT; j++)
+	inc_public(f);
+    else
+      for (int j=0; j<BLOCKCOUNT; j++)
+	inc_private(f);
+
+  return NULL;
+}
+
+static int
+t_locking(void)
+{
+  pthread_t thr[THREADS];
+  foo f = { .lock = DOMAIN_NEW(proto), };
+
+  for (int i=0; i<THREADS; i++)
+    bt_assert(pthread_create(&thr[i], NULL, thread_run, &f) == 0);
+
+  for (int i=0; i<THREADS; i++)
+    bt_assert(pthread_join(thr[i], NULL) == 0);
+
+  bt_assert(f.priv.private_counter == atomic_load_explicit(&f.counter, memory_order_relaxed));
+  bt_assert(f.priv.private_counter == THREADS * BLOCKCOUNT * REPEATS / 2);
+
+  return 1;
+}
+
+int
+main(int argc, char **argv)
+{
+  bt_init(argc, argv);
+  bt_bird_init();
+
+  bt_test_suite(t_locking, "Testing locks");
+
+  return bt_exit_value();
+}