From ca9fcec3643aef9b7cd15cc76248e0981b77decf Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Mon, 8 Jan 2018 09:49:08 +0100
Subject: [PATCH 1/4] thread/Thread: fix indent
---
src/thread/Thread.hxx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/thread/Thread.hxx b/src/thread/Thread.hxx
index 82c8196d5..f02dbf2e0 100644
--- a/src/thread/Thread.hxx
+++ b/src/thread/Thread.hxx
@@ -62,7 +62,7 @@ public:
#else
return handle != pthread_t();
#endif
- }
+ }
/**
* Check if this thread is the current thread.
From d989dbfec4a16ba8b40209aff7025645c8045681 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Mon, 8 Jan 2018 09:56:39 +0100
Subject: [PATCH 2/4] thread/Thread: make IsInside() debug-only
This method is only used inside assert().
---
src/IOThread.cxx | 4 ++++
src/IOThread.hxx | 5 +++++
src/thread/Thread.hxx | 2 ++
3 files changed, 11 insertions(+)
diff --git a/src/IOThread.cxx b/src/IOThread.cxx
index 1e3e35074..0a8e0e2ef 100644
--- a/src/IOThread.cxx
+++ b/src/IOThread.cxx
@@ -108,8 +108,12 @@ io_thread_get() noexcept
return *io.loop;
}
+#ifndef NDEBUG
+
bool
io_thread_inside() noexcept
{
return io.thread.IsInside();
}
+
+#endif
diff --git a/src/IOThread.hxx b/src/IOThread.hxx
index 63cbbee6e..341c82af7 100644
--- a/src/IOThread.hxx
+++ b/src/IOThread.hxx
@@ -20,6 +20,7 @@
#ifndef MPD_IO_THREAD_HXX
#define MPD_IO_THREAD_HXX
+#include "check.h"
#include "Compiler.h"
class EventLoop;
@@ -53,6 +54,8 @@ gcc_const
EventLoop &
io_thread_get() noexcept;
+#ifndef NDEBUG
+
/**
* Is the current thread the I/O thread?
*/
@@ -61,3 +64,5 @@ bool
io_thread_inside() noexcept;
#endif
+
+#endif
diff --git a/src/thread/Thread.hxx b/src/thread/Thread.hxx
index f02dbf2e0..797ae9676 100644
--- a/src/thread/Thread.hxx
+++ b/src/thread/Thread.hxx
@@ -64,6 +64,7 @@ public:
#endif
}
+#ifndef NDEBUG
/**
* Check if this thread is the current thread.
*/
@@ -81,6 +82,7 @@ public:
return pthread_self() == handle;
#endif
}
+#endif
void Start();
void Join();
From 2eef4e67165c38c147980ead785bcf7180b53009 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Mon, 8 Jan 2018 09:58:18 +0100
Subject: [PATCH 3/4] thread/Thread: add debug attribute "inside_handle"
This attribute shall be used only for IsInside() to make this safe
against a race condition described in #188:
> There is no requirement on the implementation that the ID of the
> created thread be available before the newly created thread starts
> executing.
http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_create.html):
This means that on some pthread implementations (e.g. Haiku), the
assert(thread.IsInside()) could fail.
Closes #188
---
NEWS | 1 +
src/thread/Thread.cxx | 4 ++++
src/thread/Thread.hxx | 12 +++++++++++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index 0f61a3aa7..f5182a945 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,5 @@
ver 0.20.16 (not yet released)
+* fix crash in debug build on Haiku and other operating systems
ver 0.20.15 (2018/01/05)
* queue: fix crash after seek failure
diff --git a/src/thread/Thread.cxx b/src/thread/Thread.cxx
index de3c46703..21aec1154 100644
--- a/src/thread/Thread.cxx
+++ b/src/thread/Thread.cxx
@@ -86,6 +86,10 @@ Thread::ThreadProc(void *ctx)
{
Thread &thread = *(Thread *)ctx;
+#ifndef NDEBUG
+ thread.inside_handle = pthread_self();
+#endif
+
thread.Run();
return nullptr;
diff --git a/src/thread/Thread.hxx b/src/thread/Thread.hxx
index 797ae9676..45250c5c7 100644
--- a/src/thread/Thread.hxx
+++ b/src/thread/Thread.hxx
@@ -41,6 +41,16 @@ class Thread {
DWORD id;
#else
pthread_t handle = pthread_t();
+
+#ifndef NDEBUG
+ /**
+ * This handle is only used by IsInside(), and is set by the
+ * thread function. Since #handle is set by pthread_create()
+ * which is racy, we need this attribute for early checks
+ * inside the thread function.
+ */
+ pthread_t inside_handle = pthread_t();
+#endif
#endif
public:
@@ -79,7 +89,7 @@ public:
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;
+ return pthread_self() == inside_handle;
#endif
}
#endif
From 1f50bdb23075b817774c9ff0d033e7395be13067 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 22 Dec 2017 11:04:24 +0100
Subject: [PATCH 4/4] event/Loop: use std::atomic_bool for the "quit" variable
Fixes thread sanitizer warnings.
---
src/event/Loop.cxx | 6 ++++--
src/event/Loop.hxx | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/event/Loop.cxx b/src/event/Loop.cxx
index ada820c2d..9e09534da 100644
--- a/src/event/Loop.cxx
+++ b/src/event/Loop.cxx
@@ -27,7 +27,7 @@
#include <algorithm>
EventLoop::EventLoop()
- :SocketMonitor(*this)
+ :SocketMonitor(*this), quit(false)
{
SocketMonitor::Open(wake_fd.Get());
SocketMonitor::Schedule(SocketMonitor::READ);
@@ -46,7 +46,9 @@ EventLoop::~EventLoop()
void
EventLoop::Break()
{
- quit = true;
+ if (quit.exchange(true))
+ return;
+
wake_fd.Write();
}
diff --git a/src/event/Loop.hxx b/src/event/Loop.hxx
index 1060a8769..cf2ec33b2 100644
--- a/src/event/Loop.hxx
+++ b/src/event/Loop.hxx
@@ -30,6 +30,7 @@
#include "SocketMonitor.hxx"
#include <chrono>
+#include <atomic>
#include <list>
#include <set>
@@ -82,7 +83,7 @@ class EventLoop final : SocketMonitor
std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
- bool quit = false;
+ std::atomic_bool quit;
/**
* True when the object has been modified and another check is