Let selector table readers share the lock

Test/CMakeLists.txt

@@ -52,6 +52,7 @@ set(TESTS
 	hash_table_delete.c
 	hash_test.c
 	setSuperclass.m
+	TypedSelectorRegistration.m
 	UnexpectedException.m
 )
 

Test/TypedSelectorRegistration.m

@@ -0,0 +1,26 @@
+#include "Test.h"
+#include <string.h>
+
+// 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;
+}

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 <pthread.h>
@@ -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__

selector_table.cc

@@ -10,6 +10,7 @@
 #include <ctype.h>
 #include <vector>
 #include <mutex>
+#include <shared_mutex>
 #include <forward_list>
 #include <tsl/robin_set.h>
 #include "class.h"
@@ -96,13 +97,18 @@ struct TypeList : public std::forward_list<const char*>
 std::vector<TypeList> *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<decltype(selector_table_lock)>;
 
+/// Type to use as a lock guard for lookup
+using ReadGuard = std::shared_lock<decltype(selector_table_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
 		{