thread/{Thread,Id}: use defaul-initialized pthread_t as "undefined" value

Use the "==" operator instead of pthread_equal().

This allows us to eliminate two boolean flags which are there to avoid
race conditions, and made the thing so fragile that I got tons of
(correct) thread sanitizer warnings.

3 files changed, 19 insertions, 50 deletions

diff --git a/src/thread/Id.hxx b/src/thread/Id.hxx
index b0bdcb421..b4ae74139 100644
--- a/src/thread/Id.hxx
+++ b/src/thread/Id.hxx
@@ -52,13 +52,11 @@ public:
 	constexpr ThreadId(pthread_t _id):id(_id) {}
 #endif
 
-	gcc_const
-	static ThreadId Null() noexcept {
+	static constexpr ThreadId Null() noexcept {
 #ifdef _WIN32
 		return 0;
 #else
-		static ThreadId null;
-		return null;
+		return pthread_t();
 #endif
 	}
 
@@ -81,11 +79,13 @@ public:
 
 	gcc_pure
 	bool operator==(const ThreadId &other) const noexcept {
-#ifdef _WIN32
+		/* note: not using pthread_equal() because that
+		   function "is undefined if either thread ID is not
+		   valid so we can't safely use it on
+		   default-constructed values" (comment from
+		   libstdc++) - and if both libstdc++ and libc++ get
+		   away with this, we can do it as well */
 		return id == other.id;
-#else
-		return pthread_equal(id, other.id);
-#endif
 	}
 
 	/**
diff --git a/src/thread/Thread.cxx b/src/thread/Thread.cxx
index 8d8a429d0..de3c46703 100644
--- a/src/thread/Thread.cxx
+++ b/src/thread/Thread.cxx
@@ -35,23 +35,10 @@ Thread::Start()
 	if (handle == nullptr)
 		throw MakeLastError("Failed to create thread");
 #else
-#ifndef NDEBUG
-	creating = true;
-#endif
-
 	int e = pthread_create(&handle, nullptr, ThreadProc, this);
 
-	if (e != 0) {
-#ifndef NDEBUG
-		creating = false;
-#endif
+	if (e != 0)
 		throw MakeErrno(e, "Failed to create thread");
-	}
-
-	defined = true;
-#ifndef NDEBUG
-	creating = false;
-#endif
 #endif
 }
 
@@ -67,23 +54,13 @@ Thread::Join()
 	handle = nullptr;
 #else
 	pthread_join(handle, nullptr);
-	defined = false;
+	handle = pthread_t();
 #endif
 }
 
 inline void
 Thread::Run()
 {
-#ifndef WIN32
-#ifndef NDEBUG
-	/* this works around a race condition that causes an assertion
-	   failure due to IsInside() spuriously returning false right
-	   after the thread has been created, and the calling thread
-	   hasn't initialised "defined" yet */
-	defined = true;
-#endif
-#endif
-
 	f();
 
 #ifdef ANDROID
diff --git a/src/thread/Thread.hxx b/src/thread/Thread.hxx
index 63f34f8e7..82c8196d5 100644
--- a/src/thread/Thread.hxx
+++ b/src/thread/Thread.hxx
@@ -40,17 +40,7 @@ class Thread {
 	HANDLE handle = nullptr;
 	DWORD id;
 #else
-	pthread_t handle;
-	bool defined = false;
-
-#ifndef NDEBUG
-	/**
-	 * The thread is currently being created.  This is a workaround for
-	 * IsInside(), which may return false until pthread_create() has
-	 * initialised the #handle.
-	 */
-	bool creating = false;
-#endif
+	pthread_t handle = pthread_t();
 #endif
 
 public:
@@ -70,7 +60,7 @@ public:
 #ifdef _WIN32
 		return handle != nullptr;
 #else
-		return defined;
+		return handle != pthread_t();
 #endif
   }
 
@@ -82,11 +72,13 @@ public:
 #ifdef _WIN32
 		return GetCurrentThreadId() == id;
 #else
-#ifdef NDEBUG
-		constexpr bool creating = false;
-#endif
-		return IsDefined() && (creating ||
-				       pthread_equal(pthread_self(), handle));
+		/* note: not using pthread_equal() because that
+		   function "is undefined if either thread ID is not
+		   valid so we can't safely use it on
+		   default-constructed values" (comment from
+		   libstdc++) - and if both libstdc++ and libc++ get
+		   away with this, we can do it as well */
+		return pthread_self() == handle;
 #endif
 	}