From 9f120e326b642c0b17b57a661f7a0cd4890a4208 Mon Sep 17 00:00:00 2001
From: Zygo Blaxell <bees@furryterror.org>
Date: Sun, 15 Jan 2017 00:30:00 -0500
Subject: [PATCH] bees: fix deadlock in thread status reporting

"s_name" was a thread_local variable, not static, and did not require a
mutex to protect access.  A deadlock is possible if a thread triggers an
exception with a handler that attempts to log a message (as the top-level
exception handler in bees does).

Remove multiple unnecessary mutex locks.  Rename the thread_local variables
to make their scope clearer.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>
---
 src/bees.cc | 39 +++++++++++++++++++--------------------
 src/bees.h  |  6 +++---
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/src/bees.cc b/src/bees.cc
index f5585d0..fd7d1de 100644
--- a/src/bees.cc
+++ b/src/bees.cc
@@ -46,7 +46,7 @@ do_cmd_help(const char **argv)
 
 RateLimiter bees_info_rate_limit(BEES_INFO_RATE, BEES_INFO_BURST);
 
-thread_local BeesTracer *BeesTracer::s_next_tracer = nullptr;
+thread_local BeesTracer *BeesTracer::tl_next_tracer = nullptr;
 
 BeesTracer::~BeesTracer()
 {
@@ -56,20 +56,20 @@ BeesTracer::~BeesTracer()
 			BEESLOG("---  END  TRACE --- exception ---");
 		}
 	}
-	s_next_tracer = m_next_tracer;
+	tl_next_tracer = m_next_tracer;
 }
 
 BeesTracer::BeesTracer(function<void()> f) :
 	m_func(f)
 {
-	m_next_tracer = s_next_tracer;
-	s_next_tracer = this;
+	m_next_tracer = tl_next_tracer;
+	tl_next_tracer = this;
 }
 
 void
 BeesTracer::trace_now()
 {
-	BeesTracer *tp = s_next_tracer;
+	BeesTracer *tp = tl_next_tracer;
 	BEESLOG("--- BEGIN TRACE ---");
 	while (tp) {
 		tp->m_func();
@@ -78,18 +78,19 @@ BeesTracer::trace_now()
 	BEESLOG("---  END  TRACE ---");
 }
 
-thread_local BeesNote *BeesNote::s_next = nullptr;
+thread_local BeesNote *BeesNote::tl_next = nullptr;
 mutex BeesNote::s_mutex;
 map<pid_t, BeesNote*> BeesNote::s_status;
-thread_local string BeesNote::s_name;
+thread_local string BeesNote::tl_name;
 
 BeesNote::~BeesNote()
 {
-	unique_lock<mutex> lock(s_mutex);
-	s_next = m_prev;
-	if (s_next) {
-		s_status[gettid()] = s_next;
+	tl_next = m_prev;
+	if (tl_next) {
+		unique_lock<mutex> lock(s_mutex);
+		s_status[gettid()] = tl_next;
 	} else {
+		unique_lock<mutex> lock(s_mutex);
 		s_status.erase(gettid());
 	}
 }
@@ -97,28 +98,26 @@ BeesNote::~BeesNote()
 BeesNote::BeesNote(function<void(ostream &os)> f) :
 	m_func(f)
 {
+	m_name = tl_name;
+	m_prev = tl_next;
+	tl_next = this;
 	unique_lock<mutex> lock(s_mutex);
-	m_name = s_name;
-	m_prev = s_next;
-	s_next = this;
-	s_status[gettid()] = s_next;
+	s_status[gettid()] = tl_next;
 }
 
 void
 BeesNote::set_name(const string &name)
 {
-	unique_lock<mutex> lock(s_mutex);
-	s_name = name;
+	tl_name = name;
 }
 
 string
 BeesNote::get_name()
 {
-	unique_lock<mutex> lock(s_mutex);
-	if (s_name.empty()) {
+	if (tl_name.empty()) {
 		return "bees";
 	} else {
-		return s_name;
+		return tl_name;
 	}
 }
 
diff --git a/src/bees.h b/src/bees.h
index d3b674e..0c68fca 100644
--- a/src/bees.h
+++ b/src/bees.h
@@ -184,7 +184,7 @@ class BeesTracer {
 	function<void()> m_func;
 	BeesTracer *m_next_tracer = 0;
 	
-	thread_local static BeesTracer *s_next_tracer;
+	thread_local static BeesTracer *tl_next_tracer;
 public:
 	BeesTracer(function<void()> f);
 	~BeesTracer();
@@ -200,8 +200,8 @@ class BeesNote {
 	static mutex			s_mutex;
 	static map<pid_t, BeesNote*>	s_status;
 
-	thread_local static BeesNote	*s_next;
-	thread_local static string	s_name;
+	thread_local static BeesNote	*tl_next;
+	thread_local static string	tl_name;
 
 public:
 	BeesNote(function<void(ostream &)> f);