gbitlock: add g_pointer_bit_unlock_and_set() and g_pointer_bit_lock_mask_ptr()

The existing g_pointer_bit_lock() and g_pointer_bit_unlock() API
requires the user to understand/reimplement how bits of the pointer get
mangled. Add helper functions for that.

The useful thing to do with g_pointer_bit_lock() API is to get/set
pointers while having it locked. For example, to set the pointer a user
can do:

    g_pointer_bit_lock (&lockptr, lock_bit);
    ptr2 = set_bit_pointer_as_if_locked(ptr, lock_bit);
    g_atomic_pointer_set (&lockptr, ptr2);
    g_pointer_bit_unlock (&lockptr, lock_bit);

That has several problems:

- it requires one extra atomic operations (3 instead of 2, in the
  non-contended case).

- the first g_atomic_pointer_set() already wakes blocked threads,
  which find themselves still being locked and needs to go back to
  sleep.

- the user needs to re-implement how bit-locking mangles the pointer so
  that it looks as if it were locked.

- while the user tries to re-implement what glib does to mangle the
  pointer for bitlocking, there is no immediate guarantee that they get
  it right.

Now we can do instead:

  g_pointer_bit_lock(&lockptr, lock_bit);
  g_pointer_bit_unlock_and_set(&lockptr, lock_bit, ptr, 0);

This will also emit a critical if @ptr has the locked bit set.
g_pointer_bit_lock() really only works with pointers that have a certain
alignment, and the lowest bits unset. Otherwise, there is no space to
encode both the locking and all pointer values. The new assertion helps
to catch such bugs.

Also, g_pointer_bit_lock_mask_ptr() is here, so we can do:

  g_pointer_bit_lock(&lockptr, lock_bit);
  /* set a pointer separately, when g_pointer_bit_unlock_and_set() is unsuitable. */
  g_atomic_pointer_set(&lockptr, g_pointer_bit_lock_mask_ptr(ptr, lock_bit, TRUE, 0, NULL));
  ...
  g_pointer_bit_unlock(&lockptr, lock_bit);

and:

  g_pointer_bit_lock(&lockptr, lock_bit);
  /* read the real pointer after getting the lock. */
  ptr = g_pointer_bit_lock_mask_ptr(lockptr, lock_bit, FALSE, 0, NULL));
  ...
  g_pointer_bit_unlock(&lockptr, lock_bit);
This commit is contained in:
Thomas Haller 2023-12-23 21:43:47 +01:00
parent f9e54fc991
commit c4c76d77cb
4 changed files with 215 additions and 6 deletions

View File

@ -387,6 +387,31 @@ g_futex_int_address (const void *address)
return int_address;
}
G_ALWAYS_INLINE static inline gpointer
pointer_bit_lock_mask_ptr (gpointer ptr, guint lock_bit, gboolean set, guintptr preserve_mask, gpointer preserve_ptr)
{
guintptr x_ptr;
guintptr x_preserve_ptr;
guintptr lock_mask;
x_ptr = (guintptr) ptr;
if (preserve_mask != 0)
{
x_preserve_ptr = (guintptr) preserve_ptr;
x_ptr = (x_preserve_ptr & preserve_mask) | (x_ptr & ~preserve_mask);
}
if (lock_bit == G_MAXUINT)
return (gpointer) x_ptr;
lock_mask = (guintptr) (1u << lock_bit);
if (set)
return (gpointer) (x_ptr | lock_mask);
else
return (gpointer) (x_ptr & ~lock_mask);
}
/**
* g_pointer_bit_lock:
* @address: (not nullable): a pointer to a #gpointer-sized value
@ -556,3 +581,90 @@ void
}
}
}
/**
* g_pointer_bit_lock_mask_ptr:
* @ptr: (nullable): the pointer to mask
* @lock_bit: the bit to set/clear. If set to `G_MAXUINT`, the
* lockbit is taken from @preserve_ptr or @ptr (depending on @preserve_mask).
* @set: whether to set (lock) the bit or unset (unlock). This
* has no effect, if @lock_bit is set to `G_MAXUINT`.
* @preserve_mask: if non-zero, a bit-mask for @preserve_ptr. The
* @preserve_mask bits from @preserve_ptr are set in the result.
* Note that the @lock_bit bit will be always set according to @set,
* regardless of @preserve_mask and @preserve_ptr (unless @lock_bit is
* `G_MAXUINT`).
* @preserve_ptr: (nullable): if @preserve_mask is non-zero, the bits
* from this pointer are set in the result.
*
* This mangles @ptr as g_pointer_bit_lock() and g_pointer_bit_unlock()
* do.
*
* Returns: the mangled pointer.
*
* Since: 2.80
**/
gpointer
g_pointer_bit_lock_mask_ptr (gpointer ptr, guint lock_bit, gboolean set, guintptr preserve_mask, gpointer preserve_ptr)
{
g_return_val_if_fail (lock_bit < 32u || lock_bit == G_MAXUINT, ptr);
return pointer_bit_lock_mask_ptr (ptr, lock_bit, set, preserve_mask, preserve_ptr);
}
/**
* g_pointer_bit_unlock_and_set:
* @address: (not nullable): a pointer to a #gpointer-sized value
* @lock_bit: a bit value between 0 and 31
* @ptr: the new pointer value to set
* @preserve_mask: if non-zero, those bits of the current pointer in @address
* are preserved.
* Note that the @lock_bit bit will be always set according to @set,
* regardless of @preserve_mask and the currently set value in @address.
*
* This is equivalent to g_pointer_bit_unlock() and atomically setting
* the pointer value.
*
* Note that the lock bit will be cleared from the pointer. If the unlocked
* pointer that was set is not identical to @ptr, an assertion fails. In other
* words, @ptr must have @lock_bit unset. This also means, you usually can
* only use this on the lowest bits.
*
* Since: 2.80
**/
void (g_pointer_bit_unlock_and_set) (void *address,
guint lock_bit,
gpointer ptr,
guintptr preserve_mask)
{
gpointer *pointer_address = address;
guint class = ((guintptr) address) % G_N_ELEMENTS (g_bit_lock_contended);
gpointer ptr2;
g_return_if_fail (lock_bit < 32u);
if (preserve_mask != 0)
{
gpointer old_ptr = g_atomic_pointer_get ((gpointer *) address);
again:
ptr2 = pointer_bit_lock_mask_ptr (ptr, lock_bit, FALSE, preserve_mask, old_ptr);
if (!g_atomic_pointer_compare_and_exchange_full (pointer_address, old_ptr, ptr2, &old_ptr))
goto again;
}
else
{
ptr2 = pointer_bit_lock_mask_ptr (ptr, lock_bit, FALSE, 0, NULL);
g_atomic_pointer_set (pointer_address, ptr2);
}
if (g_atomic_int_get (&g_bit_lock_contended[class]) > 0)
g_futex_wake (g_futex_int_address (address));
/* It makes no sense, if unlocking mangles the pointer. Assert against
* that.
*
* Note that based on @preserve_mask, the pointer also gets mangled, which
* can make sense for the caller. We don't assert for that. */
g_return_if_fail (ptr == pointer_bit_lock_mask_ptr (ptr, lock_bit, FALSE, 0, NULL));
}

View File

@ -51,6 +51,19 @@ GLIB_AVAILABLE_IN_ALL
void g_pointer_bit_unlock (volatile void *address,
gint lock_bit);
GLIB_AVAILABLE_IN_2_80
gpointer g_pointer_bit_lock_mask_ptr (gpointer ptr,
guint lock_bit,
gboolean set,
guintptr preserve_mask,
gpointer preserve_ptr);
GLIB_AVAILABLE_IN_2_80
void g_pointer_bit_unlock_and_set (void *address,
guint lock_bit,
gpointer ptr,
guintptr preserve_mask);
#ifdef __GNUC__
#define g_pointer_bit_lock(address, lock_bit) \
@ -71,6 +84,12 @@ void g_pointer_bit_unlock (volatile void *address,
g_pointer_bit_unlock ((address), (lock_bit)); \
}))
#define g_pointer_bit_unlock_and_set(address, lock_bit, ptr, preserve_mask) \
(G_GNUC_EXTENSION ({ \
G_STATIC_ASSERT (sizeof *(address) == sizeof (gpointer)); \
g_pointer_bit_unlock_and_set ((address), (lock_bit), (ptr), (preserve_mask)); \
}))
#endif
G_END_DECLS

View File

@ -43,13 +43,16 @@
#undef g_pointer_bit_lock
#undef g_pointer_bit_trylock
#undef g_pointer_bit_unlock
#undef g_pointer_bit_unlock_and_set
#define g_bit_lock _emufutex_g_bit_lock
#define g_bit_trylock _emufutex_g_bit_trylock
#define g_bit_unlock _emufutex_g_bit_unlock
#define g_pointer_bit_lock _emufutex_g_pointer_bit_lock
#define g_pointer_bit_trylock _emufutex_g_pointer_bit_trylock
#define g_pointer_bit_unlock _emufutex_g_pointer_bit_unlock
#define g_bit_lock _emufutex_g_bit_lock
#define g_bit_trylock _emufutex_g_bit_trylock
#define g_bit_unlock _emufutex_g_bit_unlock
#define g_pointer_bit_lock _emufutex_g_pointer_bit_lock
#define g_pointer_bit_trylock _emufutex_g_pointer_bit_trylock
#define g_pointer_bit_unlock _emufutex_g_pointer_bit_unlock
#define g_pointer_bit_lock_mask_ptr _emufutex_g_pointer_bit_lock_mask_ptr
#define g_pointer_bit_unlock_and_set _emufutex_g_pointer_bit_unlock_and_set
#define G_BIT_LOCK_FORCE_FUTEX_EMULATION

View File

@ -509,6 +509,79 @@ test_threaded_toggle_notify (void)
g_clear_object (&object);
}
static void
test_threaded_g_pointer_bit_unlock_and_set (void)
{
GObject *obj;
gpointer plock;
gpointer ptr;
gpointer mangled_obj;
#if defined(__GNUC__)
/* We should have at least one bit we can use safely for bit-locking */
G_STATIC_ASSERT (__alignof (GObject) > 1);
#endif
obj = g_object_new (G_TYPE_OBJECT, NULL);
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0, NULL) == obj);
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0x2, obj) == obj);
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 1, 0, NULL) != obj);
mangled_obj = obj;
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0x2, mangled_obj) == obj);
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0x3, mangled_obj) == obj);
g_atomic_pointer_and (&mangled_obj, ~((gsize) 0x7));
g_atomic_pointer_or (&mangled_obj, 0x2);
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0x2, mangled_obj) != obj);
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0x2, mangled_obj) == (gpointer) (((guintptr) obj) | ((guintptr) mangled_obj)));
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0x3, mangled_obj) == (gpointer) (((guintptr) obj) | ((guintptr) mangled_obj)));
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, TRUE, 0x3, mangled_obj) == (gpointer) (((guintptr) obj) | ((guintptr) mangled_obj) | ((guintptr) 1)));
g_atomic_pointer_and (&mangled_obj, ~((gsize) 0x2));
g_assert_true (g_pointer_bit_lock_mask_ptr (obj, 0, 0, 0x2, mangled_obj) == obj);
g_atomic_pointer_or (&mangled_obj, 0x2);
plock = obj;
g_pointer_bit_lock (&plock, 0);
g_assert_true (plock != obj);
g_pointer_bit_unlock_and_set (&plock, 0, obj, 0);
g_assert_true (plock == obj);
plock = obj;
g_pointer_bit_lock (&plock, 0);
g_assert_true (plock != obj);
g_atomic_pointer_set (&plock, mangled_obj);
g_pointer_bit_unlock_and_set (&plock, 0, obj, 0);
g_assert_true (plock == obj);
plock = obj;
g_pointer_bit_lock (&plock, 0);
g_assert_true (plock != obj);
g_atomic_pointer_set (&plock, mangled_obj);
g_pointer_bit_unlock_and_set (&plock, 0, obj, 0x7);
g_assert_true (plock != obj);
g_assert_true (plock == (gpointer) (((guintptr) obj) | ((guintptr) mangled_obj)));
plock = NULL;
g_pointer_bit_lock (&plock, 0);
g_assert_true (plock != NULL);
g_pointer_bit_unlock_and_set (&plock, 0, NULL, 0);
g_assert_true (plock == NULL);
ptr = ((char *) obj) + 1;
plock = obj;
g_pointer_bit_lock (&plock, 0);
g_assert_true (plock == ptr);
g_test_expect_message ("GLib", G_LOG_LEVEL_CRITICAL,
"*assertion 'ptr == pointer_bit_lock_mask_ptr (ptr, lock_bit, FALSE, 0, NULL)' failed*");
g_pointer_bit_unlock_and_set (&plock, 0, ptr, 0);
g_test_assert_expected_messages ();
g_assert_true (plock != ptr);
g_assert_true (plock == obj);
g_object_unref (obj);
}
int
main (int argc,
char *argv[])
@ -522,6 +595,8 @@ main (int argc,
test_threaded_weak_ref_finalization);
g_test_add_func ("/GObject/threaded-toggle-notify",
test_threaded_toggle_notify);
g_test_add_func ("/GObject/threaded-g-pointer-bit-unlock-and-set",
test_threaded_g_pointer_bit_unlock_and_set);
return g_test_run();
}