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.
This commit is contained in:
Max Kellermann 2017-12-22 10:37:07 +01:00
parent 6439727afc
commit 6a8c2848f6
3 changed files with 19 additions and 50 deletions

View File

@ -52,13 +52,11 @@ public:
constexpr ThreadId(pthread_t _id) noexcept: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
}
/**

View File

@ -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() noexcept
handle = nullptr;
#else
pthread_join(handle, nullptr);
defined = false;
handle = pthread_t();
#endif
}
inline void
Thread::Run() noexcept
{
#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

View File

@ -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
}