Skip to content

[th/g-pointer-bit-lock-ext] glib: add g_pointer_bit_unlock_and_set() and g_pointer_bit_lock_mask_ptr()

Thomas Haller requested to merge th/g-pointer-bit-lock-ext into main

The only 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);

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));
  ...
  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));
  ...
  g_pointer_bit_unlock(&lockptr, lock_bit);

Merge request reports