diff --git a/glib/gbitlock.c b/glib/gbitlock.c index eeeaabc8f..f96cae476 100644 --- a/glib/gbitlock.c +++ b/glib/gbitlock.c @@ -174,6 +174,12 @@ g_futex_wake (const gint *address) #define CONTENTION_CLASSES 11 static gint g_bit_lock_contended[CONTENTION_CLASSES]; /* (atomic) */ +G_ALWAYS_INLINE static inline guint +bit_lock_contended_class (gpointer address) +{ + return ((gsize) address) % G_N_ELEMENTS (g_bit_lock_contended); +} + #if (defined (i386) || defined (__amd64__)) #if G_GNUC_CHECK_VERSION(4, 5) #define USE_ASM_GOTO 1 @@ -226,7 +232,7 @@ g_bit_lock (volatile gint *address, v = (guint) g_atomic_int_get (address_nonvolatile); if (v & mask) { - guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended); + guint class = bit_lock_contended_class (address_nonvolatile); g_atomic_int_add (&g_bit_lock_contended[class], +1); g_futex_wait (address_nonvolatile, v); @@ -243,7 +249,7 @@ g_bit_lock (volatile gint *address, if (v & mask) /* already locked */ { - guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended); + guint class = bit_lock_contended_class (address_nonvolatile); g_atomic_int_add (&g_bit_lock_contended[class], +1); g_futex_wait (address_nonvolatile, v); @@ -337,7 +343,7 @@ g_bit_unlock (volatile gint *address, #endif { - guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended); + guint class = bit_lock_contended_class (address_nonvolatile); if (g_atomic_int_get (&g_bit_lock_contended[class])) g_futex_wake (address_nonvolatile); @@ -387,6 +393,101 @@ 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_and_get: + * @address: (not nullable): a pointer to a #gpointer-sized value + * @lock_bit: a bit value between 0 and 31 + * @out_ptr: (out) (optional): returns the set pointer atomically. + * This is the value after setting the lock, it thus always has the + * lock bit set, while previously @address had the lockbit unset. + * You may also use g_pointer_bit_lock_mask_ptr() to clear the lock bit. + * + * This is equivalent to g_bit_lock, but working on pointers (or other + * pointer-sized values). + * + * For portability reasons, you may only lock on the bottom 32 bits of + * the pointer. + * + * Since: 2.80 + **/ +void +(g_pointer_bit_lock_and_get) (gpointer address, + guint lock_bit, + guintptr *out_ptr) +{ + guint class = bit_lock_contended_class (address); + guintptr mask; + guintptr v; + + g_return_if_fail (lock_bit < 32); + + mask = 1u << lock_bit; + +#ifdef USE_ASM_GOTO + if (G_LIKELY (!out_ptr)) + { + while (TRUE) + { + __asm__ volatile goto ("lock bts %1, (%0)\n" + "jc %l[contended]" + : /* no output */ + : "r"(address), "r"((gsize) lock_bit) + : "cc", "memory" + : contended); + return; + + contended: + v = (guintptr) g_atomic_pointer_get ((gpointer *) address); + if (v & mask) + { + g_atomic_int_add (&g_bit_lock_contended[class], +1); + g_futex_wait (g_futex_int_address (address), v); + g_atomic_int_add (&g_bit_lock_contended[class], -1); + } + } + } +#endif + +retry: + v = g_atomic_pointer_or ((gpointer *) address, mask); + if (v & mask) + /* already locked */ + { + g_atomic_int_add (&g_bit_lock_contended[class], +1); + g_futex_wait (g_futex_int_address (address), (guint) v); + g_atomic_int_add (&g_bit_lock_contended[class], -1); + goto retry; + } + + if (out_ptr) + *out_ptr = (v | mask); +} + /** * g_pointer_bit_lock: * @address: (not nullable): a pointer to a #gpointer-sized value @@ -405,60 +506,9 @@ g_futex_int_address (const void *address) **/ void (g_pointer_bit_lock) (volatile void *address, - gint lock_bit) + gint lock_bit) { - void *address_nonvolatile = (void *) address; - - g_return_if_fail (lock_bit < 32); - - { -#ifdef USE_ASM_GOTO - retry: - __asm__ volatile goto ("lock bts %1, (%0)\n" - "jc %l[contended]" - : /* no output */ - : "r" (address), "r" ((gsize) lock_bit) - : "cc", "memory" - : contended); - return; - - contended: - { - gpointer *pointer_address = address_nonvolatile; - gsize mask = 1u << lock_bit; - gsize v; - - v = (gsize) g_atomic_pointer_get (pointer_address); - if (v & mask) - { - guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended); - - g_atomic_int_add (&g_bit_lock_contended[class], +1); - g_futex_wait (g_futex_int_address (address_nonvolatile), v); - g_atomic_int_add (&g_bit_lock_contended[class], -1); - } - } - goto retry; -#else - gpointer *pointer_address = address_nonvolatile; - gsize mask = 1u << lock_bit; - guintptr v; - - retry: - v = g_atomic_pointer_or (pointer_address, mask); - if (v & mask) - /* already locked */ - { - guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended); - - g_atomic_int_add (&g_bit_lock_contended[class], +1); - g_futex_wait (g_futex_int_address (address_nonvolatile), (guint) v); - g_atomic_int_add (&g_bit_lock_contended[class], -1); - - goto retry; - } -#endif - } + g_pointer_bit_lock_and_get ((gpointer *) address, (guint) lock_bit, NULL); } /** @@ -550,9 +600,97 @@ void #endif { - guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended); + guint class = bit_lock_contended_class (address_nonvolatile); + if (g_atomic_int_get (&g_bit_lock_contended[class])) g_futex_wake (g_futex_int_address (address_nonvolatile)); } } } + +/** + * 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 = bit_lock_contended_class (address); + 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)); +} diff --git a/glib/gbitlock.h b/glib/gbitlock.h index bef2c0926..f44a52c37 100644 --- a/glib/gbitlock.h +++ b/glib/gbitlock.h @@ -44,6 +44,12 @@ void g_bit_unlock (volatile gint *address, GLIB_AVAILABLE_IN_ALL void g_pointer_bit_lock (volatile void *address, gint lock_bit); + +GLIB_AVAILABLE_IN_2_80 +void g_pointer_bit_lock_and_get (gpointer address, + guint lock_bit, + guintptr *out_ptr); + GLIB_AVAILABLE_IN_ALL gboolean g_pointer_bit_trylock (volatile void *address, gint lock_bit); @@ -51,6 +57,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) \ @@ -59,6 +78,12 @@ void g_pointer_bit_unlock (volatile void *address, g_pointer_bit_lock ((address), (lock_bit)); \ })) +#define g_pointer_bit_lock_and_get(address, lock_bit, out_ptr) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(address) == sizeof (gpointer)); \ + g_pointer_bit_lock_and_get ((address), (lock_bit), (out_ptr)); \ + })) + #define g_pointer_bit_trylock(address, lock_bit) \ (G_GNUC_EXTENSION ({ \ G_STATIC_ASSERT (sizeof *(address) == sizeof (gpointer)); \ @@ -71,6 +96,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 diff --git a/glib/gdataset.c b/glib/gdataset.c index 2d6735a3a..12c77b959 100644 --- a/glib/gdataset.c +++ b/glib/gdataset.c @@ -67,15 +67,18 @@ #define G_DATALIST_FLAGS_MASK_INTERNAL 0x7 +#define G_DATALIST_CLEAN_POINTER(ptr) \ + ((GData *) ((gpointer) (((guintptr) (ptr)) & ~((guintptr) G_DATALIST_FLAGS_MASK_INTERNAL)))) + /* datalist pointer accesses have to be carried out atomically */ -#define G_DATALIST_GET_POINTER(datalist) \ - ((GData*) ((guintptr) g_atomic_pointer_get (datalist) & ~(gsize) G_DATALIST_FLAGS_MASK_INTERNAL)) +#define G_DATALIST_GET_POINTER(datalist) \ + G_DATALIST_CLEAN_POINTER (g_atomic_pointer_get (datalist)) #define G_DATALIST_SET_POINTER(datalist, pointer) G_STMT_START { \ gpointer _oldv = g_atomic_pointer_get (datalist); \ gpointer _newv; \ do { \ - _newv = (gpointer) (((gsize) _oldv & G_DATALIST_FLAGS_MASK_INTERNAL) | (guintptr) pointer); \ + _newv = (gpointer) (((guintptr) _oldv & ((guintptr) G_DATALIST_FLAGS_MASK_INTERNAL)) | (guintptr) pointer); \ } while (!g_atomic_pointer_compare_and_exchange_full ((void**) datalist, _oldv, \ _newv, &_oldv)); \ } G_STMT_END @@ -104,7 +107,6 @@ struct _GDataset /* --- prototypes --- */ static inline GDataset* g_dataset_lookup (gconstpointer dataset_location); -static inline void g_datalist_clear_i (GData **datalist); static void g_dataset_destroy_internal (GDataset *dataset); static inline gpointer g_data_set_internal (GData **datalist, GQuark key_id, @@ -134,10 +136,13 @@ static GDataset *g_dataset_cached = NULL; /* should this be #define DATALIST_LOCK_BIT 2 -static void -g_datalist_lock (GData **datalist) +G_ALWAYS_INLINE static inline GData * +g_datalist_lock_and_get (GData **datalist) { - g_pointer_bit_lock ((void **)datalist, DATALIST_LOCK_BIT); + guintptr ptr; + + g_pointer_bit_lock_and_get ((void **) datalist, DATALIST_LOCK_BIT, &ptr); + return G_DATALIST_CLEAN_POINTER (ptr); } static void @@ -146,31 +151,70 @@ g_datalist_unlock (GData **datalist) g_pointer_bit_unlock ((void **)datalist, DATALIST_LOCK_BIT); } -/* Called with the datalist lock held, or the dataset global - * lock for dataset lists - */ static void -g_datalist_clear_i (GData **datalist) +g_datalist_unlock_and_set (GData **datalist, gpointer ptr) { - GData *data; - guint i; + g_pointer_bit_unlock_and_set ((void **) datalist, DATALIST_LOCK_BIT, ptr, G_DATALIST_FLAGS_MASK_INTERNAL); +} - data = G_DATALIST_GET_POINTER (datalist); - G_DATALIST_SET_POINTER (datalist, NULL); +static gboolean +datalist_append (GData **data, GQuark key_id, gpointer new_data, GDestroyNotify destroy_func) +{ + gboolean reallocated; + GData *d; + + d = *data; + + if (!d) + { + d = g_malloc (sizeof (GData)); + d->len = 0; + d->alloc = 1; + *data = d; + reallocated = TRUE; + } + else if (d->len == d->alloc) + { + d->alloc = d->alloc * 2u; + d = g_realloc (d, G_STRUCT_OFFSET (GData, data) + d->alloc * sizeof (GDataElt)); + *data = d; + reallocated = TRUE; + } + else + reallocated = FALSE; + + d->data[d->len] = (GDataElt){ + .key = key_id, + .data = new_data, + .destroy = destroy_func, + }; + d->len++; + + return reallocated; +} + +static GDataElt * +datalist_find (GData *data, GQuark key_id, guint32 *out_idx) +{ + guint32 i; if (data) { - G_UNLOCK (g_dataset_global); for (i = 0; i < data->len; i++) { - if (data->data[i].data && data->data[i].destroy) - data->data[i].destroy (data->data[i].data); + GDataElt *data_elt = &data->data[i]; + + if (data_elt->key == key_id) + { + if (out_idx) + *out_idx = i; + return data_elt; + } } - G_LOCK (g_dataset_global); - - g_free (data); } - + if (out_idx) + *out_idx = G_MAXUINT32; + return NULL; } /** @@ -189,12 +233,8 @@ g_datalist_clear (GData **datalist) g_return_if_fail (datalist != NULL); - g_datalist_lock (datalist); - - data = G_DATALIST_GET_POINTER (datalist); - G_DATALIST_SET_POINTER (datalist, NULL); - - g_datalist_unlock (datalist); + data = g_datalist_lock_and_get (datalist); + g_datalist_unlock_and_set (datalist, NULL); if (data) { @@ -233,7 +273,12 @@ g_dataset_destroy_internal (GDataset *dataset) dataset_location = dataset->location; while (dataset) { - if (G_DATALIST_GET_POINTER(&dataset->datalist) == NULL) + GData *data; + guint i; + + data = G_DATALIST_GET_POINTER (&dataset->datalist); + + if (!data) { if (dataset == g_dataset_cached) g_dataset_cached = NULL; @@ -241,8 +286,19 @@ g_dataset_destroy_internal (GDataset *dataset) g_slice_free (GDataset, dataset); break; } - - g_datalist_clear_i (&dataset->datalist); + + G_DATALIST_SET_POINTER (&dataset->datalist, NULL); + + G_UNLOCK (g_dataset_global); + + for (i = 0; i < data->len; i++) + { + if (data->data[i].data && data->data[i].destroy) + data->data[i].destroy (data->data[i].data); + } + g_free (data); + + G_LOCK (g_dataset_global); dataset = g_dataset_lookup (dataset_location); } } @@ -279,138 +335,107 @@ g_data_set_internal (GData **datalist, GDestroyNotify new_destroy_func, GDataset *dataset) { - GData *d, *old_d; - GDataElt old, *data, *data_last, *data_end; + GData *d; + GData *new_d = NULL; + GDataElt old, *data; + guint32 idx; - g_datalist_lock (datalist); + d = g_datalist_lock_and_get (datalist); - d = G_DATALIST_GET_POINTER (datalist); + data = datalist_find (d, key_id, &idx); if (new_data == NULL) /* remove */ { - if (d) - { - data = d->data; - data_last = data + d->len - 1; - while (data <= data_last) - { - if (data->key == key_id) - { - old = *data; - if (data != data_last) - *data = *data_last; - d->len--; + if (data) + { + old = *data; + if (idx != d->len - 1u) + *data = d->data[d->len - 1u]; + d->len--; - /* We don't bother to shrink, but if all data are now gone - * we at least free the memory - */ - if (d->len == 0) - { - G_DATALIST_SET_POINTER (datalist, NULL); - g_free (d); - /* datalist may be situated in dataset, so must not be - * unlocked after we free it - */ - g_datalist_unlock (datalist); + /* We don't bother to shrink, but if all data are now gone + * we at least free the memory + */ + if (d->len == 0) + { + /* datalist may be situated in dataset, so must not be + * unlocked when we free it + */ + g_datalist_unlock_and_set (datalist, NULL); - /* the dataset destruction *must* be done - * prior to invocation of the data destroy function - */ - if (dataset) - g_dataset_destroy_internal (dataset); - } - else - { - g_datalist_unlock (datalist); - } + g_free (d); - /* We found and removed an old value - * the GData struct *must* already be unlinked - * when invoking the destroy function. - * we use (new_data==NULL && new_destroy_func!=NULL) as - * a special hint combination to "steal" - * data without destroy notification - */ - if (old.destroy && !new_destroy_func) - { - if (dataset) - G_UNLOCK (g_dataset_global); - old.destroy (old.data); - if (dataset) - G_LOCK (g_dataset_global); - old.data = NULL; - } + /* the dataset destruction *must* be done + * prior to invocation of the data destroy function + */ + if (dataset) + g_dataset_destroy_internal (dataset); + } + else + { + g_datalist_unlock (datalist); + } - return old.data; - } - data++; - } - } + /* We found and removed an old value + * the GData struct *must* already be unlinked + * when invoking the destroy function. + * we use (new_data==NULL && new_destroy_func!=NULL) as + * a special hint combination to "steal" + * data without destroy notification + */ + if (old.destroy && !new_destroy_func) + { + if (dataset) + G_UNLOCK (g_dataset_global); + old.destroy (old.data); + if (dataset) + G_LOCK (g_dataset_global); + old.data = NULL; + } + + return old.data; + } } else { - old.data = NULL; - if (d) - { - data = d->data; - data_end = data + d->len; - while (data < data_end) - { - if (data->key == key_id) - { - if (!data->destroy) - { - data->data = new_data; - data->destroy = new_destroy_func; - g_datalist_unlock (datalist); - } - else - { - old = *data; - data->data = new_data; - data->destroy = new_destroy_func; + if (data) + { + if (!data->destroy) + { + data->data = new_data; + data->destroy = new_destroy_func; + g_datalist_unlock (datalist); + } + else + { + old = *data; + data->data = new_data; + data->destroy = new_destroy_func; - g_datalist_unlock (datalist); + g_datalist_unlock (datalist); - /* We found and replaced an old value - * the GData struct *must* already be unlinked - * when invoking the destroy function. - */ - if (dataset) - G_UNLOCK (g_dataset_global); - old.destroy (old.data); - if (dataset) - G_LOCK (g_dataset_global); - } - return NULL; - } - data++; - } - } + /* We found and replaced an old value + * the GData struct *must* already be unlinked + * when invoking the destroy function. + */ + if (dataset) + G_UNLOCK (g_dataset_global); + old.destroy (old.data); + if (dataset) + G_LOCK (g_dataset_global); + } + return NULL; + } /* The key was not found, insert it */ - old_d = d; - if (d == NULL) - { - d = g_malloc (sizeof (GData)); - d->len = 0; - d->alloc = 1; - } - else if (d->len == d->alloc) - { - d->alloc = d->alloc * 2; - d = g_realloc (d, sizeof (GData) + (d->alloc - 1) * sizeof (GDataElt)); - } - if (old_d != d) - G_DATALIST_SET_POINTER (datalist, d); - - d->data[d->len].key = key_id; - d->data[d->len].data = new_data; - d->data[d->len].destroy = new_destroy_func; - d->len++; + if (datalist_append (&d, key_id, new_data, new_destroy_func)) + new_d = d; } - g_datalist_unlock (datalist); + if (new_d) + g_datalist_unlock_and_set (datalist, new_d); + else + g_datalist_unlock (datalist); return NULL; @@ -422,87 +447,104 @@ g_data_remove_internal (GData **datalist, gsize n_keys) { GData *d; + GDataElt *old; + GDataElt *old_to_free = NULL; + GDataElt *data; + GDataElt *data_end; + gsize found_keys; + gboolean free_d = FALSE; - g_datalist_lock (datalist); + d = g_datalist_lock_and_get (datalist); - d = G_DATALIST_GET_POINTER (datalist); - - if (d) + if (!d) { - GDataElt *old, *data, *data_end; - gsize found_keys; + g_datalist_unlock (datalist); + return; + } - /* Allocate an array of GDataElt to hold copies of the elements - * that are removed from the datalist. Allow enough space for all - * the keys; if a key is not found, the corresponding element of - * old is not populated, so we initialize them all to NULL to - * detect that case. */ - old = g_newa0 (GDataElt, n_keys); + /* Allocate an array of GDataElt to hold copies of the elements + * that are removed from the datalist. Allow enough space for all + * the keys; if a key is not found, the corresponding element of + * old is not populated, so we initialize them all to NULL to + * detect that case. + * + * At most allocate 400 bytes on the stack. Especially since we call + * out to external code, we don't know how much stack we can use. */ + if (n_keys <= 400u / sizeof (GDataElt)) + old = g_newa0 (GDataElt, n_keys); + else + { + old_to_free = g_new0 (GDataElt, n_keys); + old = old_to_free; + } - data = d->data; - data_end = data + d->len; - found_keys = 0; + data = d->data; + data_end = data + d->len; + found_keys = 0; - while (data < data_end && found_keys < n_keys) + while (data < data_end && found_keys < n_keys) + { + gboolean remove = FALSE; + + for (gsize i = 0; i < n_keys; i++) { - gboolean remove = FALSE; - - for (gsize i = 0; i < n_keys; i++) + if (data->key == keys[i]) { - if (data->key == keys[i]) - { - old[i] = *data; - remove = TRUE; - break; - } - } - - if (remove) - { - GDataElt *data_last = data_end - 1; - - found_keys++; - - if (data < data_last) - *data = *data_last; - - data_end--; - d->len--; - - /* We don't bother to shrink, but if all data are now gone - * we at least free the memory - */ - if (d->len == 0) - { - G_DATALIST_SET_POINTER (datalist, NULL); - g_free (d); - break; - } - } - else - { - data++; + old[i] = *data; + remove = TRUE; + break; } } - if (found_keys > 0) + if (remove) { - g_datalist_unlock (datalist); + GDataElt *data_last = data_end - 1; - for (gsize i = 0; i < n_keys; i++) + found_keys++; + + if (data < data_last) + *data = *data_last; + + data_end--; + d->len--; + + /* We don't bother to shrink, but if all data are now gone + * we at least free the memory + */ + if (d->len == 0) { - /* If keys[i] was not found, then old[i].destroy is NULL. - * Call old[i].destroy() only if keys[i] was found, and - * is associated with a destroy notifier: */ - if (old[i].destroy) - old[i].destroy (old[i].data); + free_d = TRUE; + break; } - - return; + } + else + { + data++; } } - g_datalist_unlock (datalist); + if (free_d) + { + g_datalist_unlock_and_set (datalist, NULL); + g_free (d); + } + else + g_datalist_unlock (datalist); + + if (found_keys > 0) + { + for (gsize i = 0; i < n_keys; i++) + { + /* If keys[i] was not found, then old[i].destroy is NULL. + * Call old[i].destroy() only if keys[i] was found, and + * is associated with a destroy notifier: */ + if (old[i].destroy) + old[i].destroy (old[i].data); + } + } + + if (G_UNLIKELY (old_to_free)) + g_free (old_to_free); } /** @@ -895,26 +937,13 @@ g_datalist_id_dup_data (GData **datalist, gpointer val = NULL; gpointer retval = NULL; GData *d; - GDataElt *data, *data_end; + GDataElt *data; - g_datalist_lock (datalist); + d = g_datalist_lock_and_get (datalist); - d = G_DATALIST_GET_POINTER (datalist); - if (d) - { - data = d->data; - data_end = data + d->len; - do - { - if (data->key == key_id) - { - val = data->data; - break; - } - data++; - } - while (data < data_end); - } + data = datalist_find (d, key_id, NULL); + if (data) + val = data->data; if (dup_func) retval = dup_func (val, user_data); @@ -964,7 +993,11 @@ g_datalist_id_replace_data (GData **datalist, { gpointer val = NULL; GData *d; - GDataElt *data, *data_end; + GData *new_d = NULL; + GDataElt *data; + gboolean free_d = FALSE; + gboolean set_new_d = FALSE; + guint32 idx; g_return_val_if_fail (datalist != NULL, FALSE); g_return_val_if_fail (key_id != 0, FALSE); @@ -972,76 +1005,55 @@ g_datalist_id_replace_data (GData **datalist, if (old_destroy) *old_destroy = NULL; - g_datalist_lock (datalist); + d = g_datalist_lock_and_get (datalist); - d = G_DATALIST_GET_POINTER (datalist); - if (d) + data = datalist_find (d, key_id, &idx); + if (data) { - data = d->data; - data_end = data + d->len - 1; - while (data <= data_end) + val = data->data; + if (val == oldval) { - if (data->key == key_id) + if (old_destroy) + *old_destroy = data->destroy; + if (newval != NULL) { - val = data->data; - if (val == oldval) - { - if (old_destroy) - *old_destroy = data->destroy; - if (newval != NULL) - { - data->data = newval; - data->destroy = destroy; - } - else - { - if (data != data_end) - *data = *data_end; - d->len--; - - /* We don't bother to shrink, but if all data are now gone - * we at least free the memory - */ - if (d->len == 0) - { - G_DATALIST_SET_POINTER (datalist, NULL); - g_free (d); - } - } - } - break; + data->data = newval; + data->destroy = destroy; + } + else + { + if (idx != d->len - 1u) + *data = d->data[d->len - 1u]; + d->len--; + + /* We don't bother to shrink, but if all data are now gone + * we at least free the memory + */ + if (d->len == 0) + { + set_new_d = TRUE; + free_d = TRUE; + } } - data++; } } if (val == NULL && oldval == NULL && newval != NULL) { - GData *old_d; - - /* insert newval */ - old_d = d; - if (d == NULL) - { - d = g_malloc (sizeof (GData)); - d->len = 0; - d->alloc = 1; - } - else if (d->len == d->alloc) + if (datalist_append (&d, key_id, newval, destroy)) { - d->alloc = d->alloc * 2; - d = g_realloc (d, sizeof (GData) + (d->alloc - 1) * sizeof (GDataElt)); + new_d = d; + set_new_d = TRUE; } - if (old_d != d) - G_DATALIST_SET_POINTER (datalist, d); - - d->data[d->len].key = key_id; - d->data[d->len].data = newval; - d->data[d->len].destroy = destroy; - d->len++; } - g_datalist_unlock (datalist); + if (set_new_d) + g_datalist_unlock_and_set (datalist, new_d); + else + g_datalist_unlock (datalist); + + if (free_d) + g_free (d); return val == oldval; } @@ -1067,9 +1079,7 @@ g_datalist_get_data (GData **datalist, g_return_val_if_fail (datalist != NULL, NULL); - g_datalist_lock (datalist); - - d = G_DATALIST_GET_POINTER (datalist); + d = g_datalist_lock_and_get (datalist); if (d) { data = d->data; diff --git a/glib/tests/1bit-mutex.c b/glib/tests/1bit-mutex.c index 607e3b1eb..f6a90f0bd 100644 --- a/glib/tests/1bit-mutex.c +++ b/glib/tests/1bit-mutex.c @@ -41,15 +41,20 @@ defining our own version of the g_bit_*lock symbols */ #undef g_pointer_bit_lock + #undef g_pointer_bit_lock_and_get #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_lock_and_get _emufutex_g_pointer_bit_lock_and_get + #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 diff --git a/gobject/tests/threadtests.c b/gobject/tests/threadtests.c index dee4d6e2d..ea5d6e3ca 100644 --- a/gobject/tests/threadtests.c +++ b/gobject/tests/threadtests.c @@ -509,6 +509,81 @@ 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; + guintptr ptr2; + 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_and_get (&plock, 0, &ptr2); + g_assert_true ((gpointer) ptr2 == plock); + 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_and_get (&plock, 0, NULL); + 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 +597,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(); }