diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index b88dda39..2f070b88 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -52,6 +52,7 @@ set(TESTS hash_table_delete.c hash_test.c setSuperclass.m + TypedSelectorRegistration.m UnexpectedException.m ) diff --git a/Test/TypedSelectorRegistration.m b/Test/TypedSelectorRegistration.m new file mode 100644 index 00000000..e0423ecd --- /dev/null +++ b/Test/TypedSelectorRegistration.m @@ -0,0 +1,26 @@ +#include "Test.h" +#include + +// Registering a typed selector whose untyped form is already known reads the +// selector table while the registration holds the table's lock. Both +// registrations answer the same name, and the typed one keeps its types. +int main(void) +{ + SEL untyped = sel_registerName("probeMethodWithValue:"); + assert(strcmp(sel_getName(untyped), "probeMethodWithValue:") == 0); + assert(sel_getType_np(untyped) == NULL); + + SEL typed = sel_registerTypedName_np("probeMethodWithValue:", "v@:i"); + assert(strcmp(sel_getName(typed), "probeMethodWithValue:") == 0); + assert(strcmp(sel_getType_np(typed), "v@:i") == 0); + // One copy of the name is kept, shared with the untyped selector. + assert(sel_getName(typed) == sel_getName(untyped)); + + // The same pair in the other order. + SEL typedFirst = sel_registerTypedName_np("otherProbeMethod:", "v@:d"); + SEL untypedSecond = sel_registerName("otherProbeMethod:"); + assert(strcmp(sel_getName(untypedSecond), "otherProbeMethod:") == 0); + assert(strcmp(sel_getType_np(typedFirst), "v@:d") == 0); + + return 0; +} diff --git a/lock.h b/lock.h index 071f2dce..d76ee2b2 100644 --- a/lock.h +++ b/lock.h @@ -13,6 +13,15 @@ typedef CRITICAL_SECTION mutex_t; # define LOCK(x) EnterCriticalSection(x) # define UNLOCK(x) LeaveCriticalSection(x) # define DESTROY_LOCK(x) DeleteCriticalSection(x) +// A slim reader/writer lock needs Windows Vista or later. Its exclusive mode +// is a mutex, so a writer sees the same behaviour as a critical section. +typedef SRWLOCK rwlock_t; +# define INIT_RWLOCK(x) InitializeSRWLock(&(x)) +# define RDLOCK(x) AcquireSRWLockShared(x) +# define RDUNLOCK(x) ReleaseSRWLockShared(x) +# define WRLOCK(x) AcquireSRWLockExclusive(x) +# define WRUNLOCK(x) ReleaseSRWLockExclusive(x) +# define DESTROY_RWLOCK(x) (void)(x) #else # include @@ -40,6 +49,14 @@ static inline void init_recursive_mutex(pthread_mutex_t *x) # define LOCK(x) pthread_mutex_lock(x) # define UNLOCK(x) pthread_mutex_unlock(x) # define DESTROY_LOCK(x) pthread_mutex_destroy(x) + +typedef pthread_rwlock_t rwlock_t; +# define INIT_RWLOCK(x) pthread_rwlock_init(&(x), NULL) +# define RDLOCK(x) pthread_rwlock_rdlock(x) +# define RDUNLOCK(x) pthread_rwlock_unlock(x) +# define WRLOCK(x) pthread_rwlock_wrlock(x) +# define WRUNLOCK(x) pthread_rwlock_unlock(x) +# define DESTROY_RWLOCK(x) pthread_rwlock_destroy(x) #endif __attribute__((unused)) static void objc_release_lock(void *x) @@ -113,6 +130,50 @@ class RecursiveMutex UNLOCK(&mutex); } }; + +/** + * A lock that many readers may hold at once, or one writer exclusively. It is + * not recursive: a thread that holds it must not acquire it again. + */ +class ReadWriteLock +{ + /// The underlying lock + rwlock_t rwlock; + + public: + /** + * Explicit initialisation of the underlying lock, so that this can be a + * global. + */ + void init() + { + INIT_RWLOCK(rwlock); + } + + /// Acquire the lock for writing. + void lock() + { + WRLOCK(&rwlock); + } + + /// Release the lock after writing. + void unlock() + { + WRUNLOCK(&rwlock); + } + + /// Acquire the lock for reading. + void lock_shared() + { + RDLOCK(&rwlock); + } + + /// Release the lock after reading. + void unlock_shared() + { + RDUNLOCK(&rwlock); + } +}; #endif #endif // __LIBOBJC_LOCK_H_INCLUDED__ diff --git a/selector_table.cc b/selector_table.cc index 5fc68af8..bc57ecee 100644 --- a/selector_table.cc +++ b/selector_table.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include "class.h" @@ -96,13 +97,18 @@ struct TypeList : public std::forward_list std::vector *selector_list; /** - * Lock protecting the selector table. + * Lock protecting the selector table. Registration is the only writer, so + * lookups share it. It is not recursive: anything called with it held must + * use the _locked form. */ -RecursiveMutex selector_table_lock; +ReadWriteLock selector_table_lock; -/// Type to use as a lock guard +/// Type to use as a lock guard for registration using LockGuard = std::lock_guard; +/// Type to use as a lock guard for lookup +using ReadGuard = std::shared_lock; + inline TypeList *selLookup_locked(uint32_t idx) { if (idx >= selector_list->size()) @@ -114,7 +120,7 @@ inline TypeList *selLookup_locked(uint32_t idx) inline TypeList *selLookup(uint32_t idx) { - LockGuard g{selector_table_lock}; + ReadGuard g{selector_table_lock}; return selLookup_locked(idx); } @@ -367,14 +373,19 @@ extern "C" PRIVATE void init_selector_tables() selector_table_lock.init(); } -static SEL selector_lookup(const char *name, const char *types) +static SEL selector_lookup_locked(const char *name, const char *types) { UnregisteredSelector sel = {name, types}; - LockGuard g{selector_table_lock}; auto result = selector_table->find(sel); return (result == selector_table->end()) ? nullptr : *result; } +static SEL selector_lookup(const char *name, const char *types) +{ + ReadGuard g{selector_table_lock}; + return selector_lookup_locked(name, types); +} + static inline void add_selector_to_table(SEL aSel) { // Store the name at the head of the list. @@ -404,7 +415,7 @@ static inline void register_selector_locked(SEL aSel) objc_resize_dtables(selector_list->size()); return; } - SEL untyped = selector_lookup(aSel->name, 0); + SEL untyped = selector_lookup_locked(aSel->name, 0); // If this has a type encoding, store the untyped version too. if (untyped == nullptr) { @@ -476,7 +487,7 @@ SEL objc_register_selector_copy(UnregisteredSelector &aSel, BOOL copyArgs) // registration; see objc_register_selector above and gnustep/libobjc2#391. LOCK_RUNTIME_FOR_SCOPE(); LockGuard g{selector_table_lock}; - copy = selector_lookup(aSel.name, aSel.types); + copy = selector_lookup_locked(aSel.name, aSel.types); if (nullptr != copy && selector_identical(aSel, copy)) { return copy; @@ -488,10 +499,10 @@ SEL objc_register_selector_copy(UnregisteredSelector &aSel, BOOL copyArgs) copy->types = (nullptr == aSel.types) ? nullptr : aSel.types; if (copyArgs) { - SEL untyped = selector_lookup(aSel.name, 0); + SEL untyped = selector_lookup_locked(aSel.name, 0); if (untyped != nullptr) { - copy->name = sel_getName(untyped); + copy->name = sel_getNameNonUnique(untyped); } else {