From 7b0958362972f67fcbadbe2dd422fa0cbbc09e4b Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Mon, 13 Apr 2026 17:24:49 +0100 Subject: [PATCH] Improve locking for properties. This fixes the issue where two addresses that hash to the same address could deadlock and also fixes a performance issue due to using spinlocks with no OS-mediated sleep. Initially this makes the spinlocks.h consumers compile as C++ so that they can use C++ abstractions over futexes and similar. A lot of this code called calloc, so now has some nicer C++ wrappers for that. This also includes some fixes to work around older C++ standard libraries. Also includes some small tweaks to Cirrus, though Cirrus is going away soon so we'll need an alternative for FreeBSD CI. --- .cirrus.yml | 8 +- CMakeLists.txt | 8 +- associate.m => associate.mm | 93 ++++++++-------- class.h | 3 +- dtable.c | 6 +- dtable.h | 23 ++-- helpers.hh | 16 +++ lock.h | 6 +- objcxx_eh.cc | 3 + properties.h | 9 ++ properties.m => properties.mm | 73 +++++-------- spinlock.h | 199 ++++++++++++++++++++++++++-------- 12 files changed, 283 insertions(+), 164 deletions(-) rename associate.m => associate.mm (83%) create mode 100644 helpers.hh rename properties.m => properties.mm (88%) diff --git a/.cirrus.yml b/.cirrus.yml index d6985ef3..e25000db 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -1,11 +1,9 @@ libcxxrt_freebsd_task: matrix: - freebsd_instance: - image_family: freebsd-13-5 + image_family: freebsd-15-0-amd64-zfs - freebsd_instance: - image_family: freebsd-15-0-snap - - freebsd_instance: - image_family: freebsd-14-2 + image_family: freebsd-14-3 install_script: pkg install -y cmake ninja git @@ -31,7 +29,7 @@ libcxxrt_freebsd_task: libcxxrt_master_task: freebsd_instance: - image_family: freebsd-14-2 + image_family: freebsd-14-3 install_script: pkg install -y cmake ninja git clone_script: | diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d915d1f..c5a0fbdb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -55,13 +55,13 @@ add_compile_definitions(GNUSTEP __OBJC_RUNTIME_INTERNAL__=1 __OBJC_BOOL) set(libobjc_ASM_SRCS objc_msgSend.S) set(libobjc_OBJCXX_SRCS + properties.mm arc.mm + associate.mm ) set(libobjc_OBJC_SRCS NSBlocks.m - associate.m - blocks_runtime_np.m - properties.m) + blocks_runtime_np.m) set(libobjc_C_SRCS alias_table.c builtin_classes.c @@ -257,7 +257,7 @@ endif () add_library(objc SHARED ${libobjc_C_SRCS} ${libobjc_ASM_SRCS} ${libobjc_OBJC_SRCS} ${libobjc_OBJCXX_SRCS} ${libobjc_ASM_OBJS}) target_compile_options(objc PRIVATE "$<$,$>:-Wno-gnu-folding-constant;-Wno-deprecated-objc-isa-usage;-Wno-objc-root-class;-fobjc-runtime=gnustep-2.0>$<$:-Xclang;-fexceptions;-Wno-gnu-folding-constant>") -target_compile_features(objc PRIVATE cxx_std_17) +target_compile_features(objc PRIVATE cxx_std_20) list(APPEND libobjc_CXX_SRCS ${libobjcxx_CXX_SRCS}) target_sources(objc PRIVATE ${libobjc_CXX_SRCS}) diff --git a/associate.m b/associate.mm similarity index 83% rename from associate.m rename to associate.mm index 2b987f99..744b4890 100644 --- a/associate.m +++ b/associate.mm @@ -11,6 +11,7 @@ #include "selector.h" #include "lock.h" #include "gc_ops.h" +#include "helpers.hh" /** * A single associative reference. Contains the key, value, and association @@ -105,7 +106,7 @@ static void cleanupReferenceList(struct reference_list *list) // Full barrier - ensure that we've zero'd the key before doing // this! __sync_synchronize(); - objc_release(r->object); + objc_release((id)r->object); } r->object = 0; r->policy = 0; @@ -117,7 +118,7 @@ static void freeReferenceList(struct reference_list *l) { if (NULL == l) { return; } freeReferenceList(l->next); - gc->free(l); + free(l); } static void setReference(struct reference_list *list, @@ -135,44 +136,45 @@ static void setReference(struct reference_list *list, break; case OBJC_ASSOCIATION_RETAIN_NONATOMIC: case OBJC_ASSOCIATION_RETAIN: - obj = objc_retain(obj); + obj = objc_retain((id)obj); case OBJC_ASSOCIATION_ASSIGN: break; } // While inserting into the list, we need to lock it temporarily. - volatile int *lock = lock_for_pointer(list); - lock_spinlock(lock); struct reference *r = findReference(list, key); - // If there's an existing reference, then we can update it, otherwise we - // have to install a new one - if (NULL == r) { - // Search for an unused slot - r = findReference(list, 0); + auto lock = acquire_locks_for_pointers(list); + // If there's an existing reference, then we can update it, otherwise we + // have to install a new one if (NULL == r) { - struct reference_list *l = list; + // Search for an unused slot + r = findReference(list, 0); + if (NULL == r) + { + struct reference_list *l = list; - while (NULL != l->next) { l = l->next; } + while (NULL != l->next) { l = l->next; } - l->next = gc->malloc(sizeof(struct reference_list)); - r = &l->next->list[0]; + l->next = allocate_zeroed(); + r = &l->next->list[0]; + } + r->key = key; } - r->key = key; } - unlock_spinlock(lock); // Now we only need to lock if the old or new property is atomic BOOL needLock = isAtomic(r->policy) || isAtomic(policy); + ThinLock *lock; if (needLock) { lock = lock_for_pointer(r); - lock_spinlock(lock); + lock->lock(); } @try { if (OBJC_ASSOCIATION_ASSIGN != r->policy) { - objc_release(r->object); + objc_release((id)r->object); } } @finally @@ -182,7 +184,7 @@ static void setReference(struct reference_list *list, } if (needLock) { - unlock_spinlock(lock); + lock->unlock(); } } @@ -201,8 +203,8 @@ static inline Class findHiddenClass(id obj) static Class allocateHiddenClass(Class superclass) { - Class newClass = - calloc(1, sizeof(struct objc_class) + sizeof(struct reference_list)); + struct objc_class *newClass = + allocate_zeroed(sizeof(struct reference_list)); if (Nil == newClass) { return Nil; } @@ -221,9 +223,9 @@ static Class allocateHiddenClass(Class superclass) LOCK_RUNTIME_FOR_SCOPE(); newClass->sibling_class = superclass->subclass_list; - superclass->subclass_list = newClass; + superclass->subclass_list = (Class)newClass; - return newClass; + return (Class)newClass; } static inline Class initHiddenClassForObject(id obj) @@ -248,7 +250,7 @@ static void deallocHiddenClass(id obj, SEL _cmd) Class hiddenClass = findHiddenClass(obj); // After calling [super dealloc], the object will no longer exist. // Free the hidden class. - struct reference_list *list = object_getIndexedIvars(hiddenClass); + struct reference_list *list = static_cast(object_getIndexedIvars(hiddenClass)); DESTROY_LOCK(&list->lock); cleanupReferenceList(list); freeReferenceList(list->next); @@ -289,19 +291,16 @@ static void deallocHiddenClass(id obj, SEL _cmd) Class cls = (Class)object; if ((NULL == cls->extra_data) && create) { - volatile int *lock = lock_for_pointer(cls); - struct reference_list *list = gc->malloc(sizeof(struct reference_list)); - lock_spinlock(lock); + struct reference_list *list = allocate_zeroed(); + auto guard = acquire_locks_for_pointers(cls); if (NULL == cls->extra_data) { INIT_LOCK(list->lock); cls->extra_data = list; - unlock_spinlock(lock); } else { - unlock_spinlock(lock); - gc->free(list); + free(list); } } return cls->extra_data; @@ -309,18 +308,16 @@ static void deallocHiddenClass(id obj, SEL _cmd) Class hiddenClass = findHiddenClass(object); if ((NULL == hiddenClass) && create) { - volatile int *lock = lock_for_pointer(object); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(object); hiddenClass = findHiddenClass(object); if (NULL == hiddenClass) { hiddenClass = initHiddenClassForObject(object); - struct reference_list *list = object_getIndexedIvars(hiddenClass); + struct reference_list *list = static_cast(object_getIndexedIvars(hiddenClass)); INIT_LOCK(list->lock); } - unlock_spinlock(lock); } - return hiddenClass ? object_getIndexedIvars(hiddenClass) : NULL; + return hiddenClass ? static_cast(object_getIndexedIvars(hiddenClass)) : nullptr; } void objc_setAssociatedObject(id object, @@ -345,9 +342,9 @@ id objc_getAssociatedObject(id object, const void *key) // Apple's objc4 retains and autoreleases the object under these policies if (r->policy & OBJC_ASSOCIATION_RETAIN_NONATOMIC) { - objc_retainAutorelease(r->object); + objc_retainAutorelease((id)r->object); } - return r->object; + return (id)r->object; } if (class_isMetaClass(object->isa)) { @@ -363,7 +360,7 @@ id objc_getAssociatedObject(id object, const void *key) } if (Nil != cls) { - struct reference_list *next_list = object_getIndexedIvars(cls); + struct reference_list *next_list = static_cast(object_getIndexedIvars(cls)); if (list != next_list) { list = next_list; @@ -372,9 +369,9 @@ id objc_getAssociatedObject(id object, const void *key) { if (r->policy & OBJC_ASSOCIATION_RETAIN_NONATOMIC) { - objc_retainAutorelease(r->object); + objc_retainAutorelease((id)r->object); } - return r->object; + return (id)r->object; } } cls = class_getSuperclass(cls); @@ -433,16 +430,14 @@ static Class hiddenClassForObject(id object) Class hiddenClass = findHiddenClass(object); if (NULL == hiddenClass) { - volatile int *lock = lock_for_pointer(object); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(object); hiddenClass = findHiddenClass(object); if (NULL == hiddenClass) { hiddenClass = initHiddenClassForObject(object); - struct reference_list *list = object_getIndexedIvars(hiddenClass); + struct reference_list *list = static_cast(object_getIndexedIvars(hiddenClass)); INIT_LOCK(list->lock); } - unlock_spinlock(lock); } return hiddenClass; } @@ -464,13 +459,13 @@ id object_clone_np(id object) // Make sure that the prototype has a hidden class, so that methods added // to it will appear in the clone. referenceListForObject(object, YES); - id new = class_createInstance(object->isa, 0); - Class hiddenClass = initHiddenClassForObject(new); - struct reference_list *list = object_getIndexedIvars(hiddenClass); + id newInstance = class_createInstance(object->isa, 0); + Class hiddenClass = initHiddenClassForObject(newInstance); + struct reference_list *list = static_cast(object_getIndexedIvars(hiddenClass)); INIT_LOCK(list->lock); - objc_setAssociatedObject(new, &prototypeKey, object, + objc_setAssociatedObject(newInstance, &prototypeKey, object, OBJC_ASSOCIATION_RETAIN_NONATOMIC); - return new; + return newInstance; } id object_getPrototype_np(id object) diff --git a/class.h b/class.h index 081feab4..b25d5ddf 100644 --- a/class.h +++ b/class.h @@ -2,6 +2,7 @@ #define __OBJC_CLASS_H_INCLUDED #include "visibility.h" #include "objc/runtime.h" +#include "sarray2.h" #include #ifdef __cplusplus @@ -96,7 +97,7 @@ struct objc_class * The dispatch table for this class. Intialized and maintained by the * runtime. */ - void *dtable; + SparseArray *dtable; /** * A pointer to the first subclass for this class. Filled in by the * runtime. diff --git a/dtable.c b/dtable.c index fb7b1bd7..54c0d729 100644 --- a/dtable.c +++ b/dtable.c @@ -696,8 +696,8 @@ static void remove_dtable(InitializingDtable* meta_buffer) LOCK(&initialize_lock); InitializingDtable *buffer = meta_buffer->next; // Install the dtable: - meta_buffer->class->dtable = meta_buffer->dtable; - buffer->class->dtable = buffer->dtable; + meta_buffer->owner->dtable = meta_buffer->dtable; + buffer->owner->dtable = buffer->dtable; // Remove the look-aside buffer entry. if (temporary_dtables == meta_buffer) { @@ -706,7 +706,7 @@ static void remove_dtable(InitializingDtable* meta_buffer) else { InitializingDtable *prev = temporary_dtables; - while (prev->next->class != meta_buffer->class) + while (prev->next->owner != meta_buffer->owner) { prev = prev->next; } diff --git a/dtable.h b/dtable.h index f69ee770..db70e96d 100644 --- a/dtable.h +++ b/dtable.h @@ -6,12 +6,14 @@ #include #include -#ifdef __OBJC_LOW_MEMORY__ -typedef struct objc_dtable* dtable_t; -struct objc_slot* objc_dtable_lookup(dtable_t dtable, uint32_t uid); -#else typedef SparseArray* dtable_t; -# define objc_dtable_lookup SparseArrayLookup +#define objc_dtable_lookup SparseArrayLookup + +typedef struct objc_class *Class; + +#ifdef __cplusplus +extern "C" +{ #endif /** @@ -27,7 +29,7 @@ PRIVATE extern dtable_t uninstalled_dtable; typedef struct _InitializingDtable { /** The class that owns the dtable. */ - Class class; + struct objc_class *owner; /** The dtable for this class. */ dtable_t dtable; /** The next uninstalled dtable in the list. */ @@ -50,12 +52,13 @@ OBJC_PUBLIC int objc_sync_enter(id object); OBJC_PUBLIC int objc_sync_exit(id object); + /** * Returns the dtable for a given class. If we are currently in an +initialize * method then this will block if called from a thread other than the one * running the +initialize method. */ -static inline dtable_t dtable_for_class(Class cls) +static inline dtable_t dtable_for_class(struct objc_class *cls) { if (classHasInstalledDtable(cls)) { @@ -77,7 +80,7 @@ static inline dtable_t dtable_for_class(Class cls) InitializingDtable *buffer = temporary_dtables; while (NULL != buffer) { - if (buffer->class == cls) + if (buffer->owner == cls) { dtable = buffer->dtable; break; @@ -139,3 +142,7 @@ void free_dtable(dtable_t dtable); * is installed. */ void checkARCAccessorsSlow(Class cls); + +#ifdef __cplusplus +} +#endif diff --git a/helpers.hh b/helpers.hh new file mode 100644 index 00000000..c447c865 --- /dev/null +++ b/helpers.hh @@ -0,0 +1,16 @@ +#pragma once +#include + +template +T *allocate_zeroed(size_t extraSpace = 0) +{ + return static_cast(calloc(1, sizeof(T) + extraSpace)); +} + +template +T *allocate_zeroed_array(size_t elements) +{ + return static_cast(calloc(elements, sizeof(T))); +} + + diff --git a/lock.h b/lock.h index e5dacd3c..071f2dce 100644 --- a/lock.h +++ b/lock.h @@ -72,7 +72,11 @@ __attribute__((unused)) static void objc_release_lock(void *x) /** * The global runtime mutex. */ -extern mutex_t runtime_mutex; +extern +#ifdef __cplusplus +"C" +#endif +mutex_t runtime_mutex; #define LOCK_RUNTIME() LOCK(&runtime_mutex) #define UNLOCK_RUNTIME() UNLOCK(&runtime_mutex) diff --git a/objcxx_eh.cc b/objcxx_eh.cc index 8ef5baf3..10a29d1d 100644 --- a/objcxx_eh.cc +++ b/objcxx_eh.cc @@ -1,3 +1,6 @@ +// Prevent including libstdc++'s incompatible header. +#define _TYPEINFO + #include #include #include diff --git a/properties.h b/properties.h index 02f25113..aff0821f 100644 --- a/properties.h +++ b/properties.h @@ -223,6 +223,11 @@ static inline struct objc_property *property_at_index(struct objc_property_list return (struct objc_property*)(((char*)l->properties) + (i * l->size)); } +#ifdef __cplusplus +extern "C" +{ +#endif + /** * Constructs a property description from a list of attributes, returning the * instance variable name via the third parameter. @@ -237,3 +242,7 @@ PRIVATE struct objc_property propertyFromAttrs(const objc_property_attribute_t * */ PRIVATE const char *constructPropertyAttributes(objc_property_t property, const char *iVarName); + +#ifdef __cplusplus +} +#endif diff --git a/properties.m b/properties.mm similarity index 88% rename from properties.m rename to properties.mm index 46b2e7f8..7c8446a8 100644 --- a/properties.m +++ b/properties.mm @@ -7,12 +7,14 @@ #include "class.h" #include "properties.h" #include "spinlock.h" +#include "helpers.hh" #include "visibility.h" #include "nsobject.h" #include "gc_ops.h" #include "lock.h" -PRIVATE int spinlocks[spinlock_count]; +extern "C" +{ /** * Public function for getting a property. @@ -26,11 +28,11 @@ id objc_getProperty(id obj, SEL _cmd, ptrdiff_t offset, BOOL isAtomic) id ret; if (isAtomic) { - volatile int *lock = lock_for_pointer(addr); - lock_spinlock(lock); - ret = *(id*)addr; - ret = objc_retain(ret); - unlock_spinlock(lock); + { + auto guard = acquire_locks_for_pointers(addr); + ret = *(id*)addr; + ret = objc_retain(ret); + } ret = objc_autoreleaseReturnValue(ret); } else @@ -59,11 +61,9 @@ void objc_setProperty(id obj, SEL _cmd, ptrdiff_t offset, id arg, BOOL isAtomic, id old; if (isAtomic) { - volatile int *lock = lock_for_pointer(addr); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(addr); old = *(id*)addr; *(id*)addr = arg; - unlock_spinlock(lock); } else { @@ -79,11 +79,9 @@ void objc_setProperty_atomic(id obj, SEL _cmd, id arg, ptrdiff_t offset) char *addr = (char*)obj; addr += offset; arg = objc_retain(arg); - volatile int *lock = lock_for_pointer(addr); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(addr); id old = *(id*)addr; *(id*)addr = arg; - unlock_spinlock(lock); objc_release(old); } @@ -94,11 +92,9 @@ void objc_setProperty_atomic_copy(id obj, SEL _cmd, id arg, ptrdiff_t offset) addr += offset; arg = [arg copy]; - volatile int *lock = lock_for_pointer(addr); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(addr); id old = *(id*)addr; *(id*)addr = arg; - unlock_spinlock(lock); objc_release(old); } @@ -127,33 +123,24 @@ void objc_setProperty_nonatomic_copy(id obj, SEL _cmd, id arg, ptrdiff_t offset) void objc_copyCppObjectAtomic(void *dest, const void *src, void (*copyHelper) (void *dest, const void *source)) { - volatile int *lock = lock_for_pointer(src < dest ? src : dest); - volatile int *lock2 = lock_for_pointer(src < dest ? dest : src); - lock_spinlock(lock); - lock_spinlock(lock2); + auto guard = acquire_locks_for_pointers(src, dest); copyHelper(dest, src); - unlock_spinlock(lock); - unlock_spinlock(lock2); } OBJC_PUBLIC void objc_getCppObjectAtomic(void *dest, const void *src, void (*copyHelper) (void *dest, const void *source)) { - volatile int *lock = lock_for_pointer(src); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(src); copyHelper(dest, src); - unlock_spinlock(lock); } OBJC_PUBLIC void objc_setCppObjectAtomic(void *dest, const void *src, void (*copyHelper) (void *dest, const void *source)) { - volatile int *lock = lock_for_pointer(dest); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(dest); copyHelper(dest, src); - unlock_spinlock(lock); } /** @@ -172,13 +159,8 @@ void objc_copyPropertyStruct(void *dest, { if (atomic) { - volatile int *lock = lock_for_pointer(src < dest ? src : dest); - volatile int *lock2 = lock_for_pointer(src < dest ? dest : src); - lock_spinlock(lock); - lock_spinlock(lock2); + auto guard = acquire_locks_for_pointers(src, dest); memcpy(dest, src, size); - unlock_spinlock(lock); - unlock_spinlock(lock2); } else { @@ -199,10 +181,8 @@ void objc_getPropertyStruct(void *dest, { if (atomic) { - volatile int *lock = lock_for_pointer(src); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(src); memcpy(dest, src, size); - unlock_spinlock(lock); } else { @@ -223,10 +203,8 @@ void objc_setPropertyStruct(void *dest, { if (atomic) { - volatile int *lock = lock_for_pointer(dest); - lock_spinlock(lock); + auto guard = acquire_locks_for_pointers(dest); memcpy(dest, src, size); - unlock_spinlock(lock); } else { @@ -285,7 +263,7 @@ objc_property_t class_getProperty(Class cls, const char *name) { return NULL; } - objc_property_t *list = calloc(count, sizeof(objc_property_t)); + objc_property_t *list = allocate_zeroed_array(count); unsigned int out = 0; for (struct objc_property_list *l=properties ; NULL!=l ; l=l->next) { @@ -423,7 +401,7 @@ objc_property_t class_getProperty(Class cls, const char *name) } count++; } - objc_property_attribute_t *propAttrs = calloc(count, sizeof(objc_property_attribute_t)); + objc_property_attribute_t *propAttrs = allocate_zeroed_array(count); memcpy(propAttrs, attrs, count * sizeof(objc_property_attribute_t)); if (NULL != outCount) { @@ -484,7 +462,7 @@ objc_property_t class_getProperty(Class cls, const char *name) return NULL; } - char *buffer = malloc(attributesSize); + char *buffer = static_cast(malloc(attributesSize)); char *out = buffer; out = addAttrIfExists('T', out, attributes, attributeCount); @@ -503,9 +481,10 @@ objc_property_t class_getProperty(Class cls, const char *name) return buffer; } + PRIVATE struct objc_property propertyFromAttrs(const objc_property_attribute_t *attributes, - unsigned int attributeCount, - const char *name) + unsigned int attributeCount, + const char *name) { struct objc_property p; p.name = strdup(name); @@ -535,7 +514,6 @@ PRIVATE struct objc_property propertyFromAttrs(const objc_property_attribute_t * return p; } - OBJC_PUBLIC BOOL class_addProperty(Class cls, const char *name, @@ -546,8 +524,7 @@ BOOL class_addProperty(Class cls, struct objc_property p = propertyFromAttrs(attributes, attributeCount, name); - struct objc_property_list *l = calloc(1, sizeof(struct objc_property_list) - + sizeof(struct objc_property)); + struct objc_property_list *l = allocate_zeroed(sizeof(struct objc_property)); l->count = 1; l->size = sizeof(struct objc_property); memcpy(&l->properties, &p, sizeof(struct objc_property)); @@ -611,3 +588,5 @@ void class_replaceProperty(Class cls, } return 0; } + +} // extern "C" diff --git a/spinlock.h b/spinlock.h index 2efec475..f707d6a6 100644 --- a/spinlock.h +++ b/spinlock.h @@ -1,23 +1,157 @@ -#ifdef _WIN32 -#include "safewindows.h" -static unsigned sleep(unsigned seconds) +#include "visibility.h" +#include +#include +#include +#include + + +// Not all supported targets implement the wait / notify instructions on +// atomics. Provide simple spinning fallback for ones that don't. +template +concept Waitable = requires(T t) { - Sleep(seconds*1000); - return 0; -} -#else -#include -#endif + t.notify_all(); +}; /** - * Number of spinlocks. This allocates one page on 32-bit platforms. + * Lightweight spinlock that falls back to using the operating system's futex + * abstraction if one exists. + */ +class ThinLock +{ + // Underlying type for the lock word. Should be the type used for futexes + // and similar. + using LockWordUnderlyingType = uint32_t; + // A lock word, as a 3-state state machine. + enum LockState : LockWordUnderlyingType + { + Unlocked = 0, + Locked = 1, + LockedWithWaiters = 3 + }; + // The lock word + std::atomic lockWord; + + // Call notify if it exists + template + static void notify(T &atomic) requires (Waitable) + { + atomic.notify_all(); + } + + // Simply ignore notify if we don't have one. + template + static void notify(T &atomic) requires (!Waitable) + { + } + + // Wait if we have a wait method. + template + static void wait(T &atomic, LockState expected) requires (Waitable) + { + atomic.wait(expected); + } + + // Short sleep if we don't. + template + static void wait(T &atomic, LockState expected) requires (!Waitable) + { + using namespace std::chrono_literals; + std::this_thread::sleep_for(1ms); + } + + public: + // Acquire the lock + void lock() + { + while (true) + { + auto old = LockState::Unlocked; + // CAS in the locked state. + if (lockWord.compare_exchange_strong(old, LockState::Locked)) + { + return; + } + // If the CAS failed add the waiters flag. + if (old != LockState::LockedWithWaiters) + { + if (!lockWord.compare_exchange_strong(old, LockState::LockedWithWaiters)) + { + // If the CAS failed, this means that we lost the race, retry. + continue; + } + } + wait(lockWord, LockState::LockedWithWaiters); + } + } + + // Release the lock + void unlock() + { + auto old = lockWord.exchange(LockState::Unlocked); + if (old == LockState::LockedWithWaiters) + { + notify(lockWord); + } + } + + // Stable ordering of locks. + // This should be operator<=>, but we support targets with no + auto operator==(ThinLock &other) + { + return this == &other; + } + + auto operator<(ThinLock &other) + { + return this < &other; + } +}; + +/** + * Deadlock-free lock guard. Acquires the two locks in order defined by their + * sort order. If the two locks are the same, does not acquire the second. + */ +class DoubleLockGuard +{ + // The first lock + ThinLock *lock1; + // The second lock + ThinLock *lock2; + public: + DoubleLockGuard(ThinLock *lock1, ThinLock *lock2) : lock1(lock1), lock2(lock2) + { + if (lock2 < lock1) + { + std::swap(lock1, lock2); + } + lock1->lock(); + if (lock1 != lock2) + { + lock2->lock(); + } + } + ~DoubleLockGuard() + { + assert(lock2 >= lock1); + lock1->unlock(); + if (lock1 != lock2) + { + lock2->unlock(); + } + } +}; + +/** + * Number of spinlocks. This allocates one page with a 32-bit lock. */ #define spinlock_count (1<<10) static const int spinlock_mask = spinlock_count - 1; /** * Integers used as spinlocks for atomic property access. */ -extern int spinlocks[spinlock_count]; +PRIVATE inline ThinLock spinlocks[spinlock_count]; + /** * Get a spin lock from a pointer. We want to prevent lock contention between * properties in the same object - if someone is stupid enough to be using @@ -26,7 +160,7 @@ extern int spinlocks[spinlock_count]; * contention between the same property in different objects, so we can't just * use the ivar offset. */ -static inline volatile int *lock_for_pointer(const void *ptr) +static inline ThinLock *lock_for_pointer(const void *ptr) { intptr_t hash = (intptr_t)ptr; // Most properties will be pointers, so disregard the lowest few bits @@ -38,44 +172,17 @@ static inline volatile int *lock_for_pointer(const void *ptr) } /** - * Unlocks the spinlock. This is not an atomic operation. We are only ever - * modifying the lowest bit of the spinlock word, so it doesn't matter if this - * is two writes because there is no contention among the high bit. There is - * no possibility of contention among calls to this, because it may only be - * called by the thread owning the spin lock. + * Returns a lock guard for the lock associated with a single address. */ -inline static void unlock_spinlock(volatile int *spinlock) +inline auto acquire_locks_for_pointers(const void *ptr) { - __sync_synchronize(); - *spinlock = 0; + return std::lock_guard{*lock_for_pointer(ptr)}; } + /** - * Attempts to lock a spinlock. This is heavily optimised for the uncontended - * case, because property access should (generally) not be contended. In the - * uncontended case, this is a single atomic compare and swap instruction and a - * branch. Atomic CAS is relatively expensive (can be a pipeline flush, and - * may require locking a cache line in a cache-coherent SMP system, but it's a - * lot cheaper than a system call). - * - * If the lock is contended, then we just sleep and then try again after the - * other threads have run. Note that there is no upper bound on the potential - * running time of this function, which is one of the great many reasons that - * using atomic accessors is a terrible idea, but in the common case it should - * be very fast. + * Returns a lock guard for locks associated with two addresses. */ -inline static void lock_spinlock(volatile int *spinlock) +inline auto acquire_locks_for_pointers(const void *ptr, const void *ptr2) { - int count = 0; - // Set the spin lock value to 1 if it is 0. - while(!__sync_bool_compare_and_swap(spinlock, 0, 1)) - { - count++; - if (0 == count % 10) - { - // If it is already 1, let another thread play with the CPU for a - // bit then try again. - sleep(0); - } - } + return DoubleLockGuard(lock_for_pointer(ptr), lock_for_pointer(ptr2)); } -