From 0710208354d9ee89aceb708f9d00dc2d84c7c894 Mon Sep 17 00:00:00 2001
From: Zygo Blaxell <bees@furryterror.org>
Date: Tue, 23 Jan 2018 00:06:33 -0500
Subject: [PATCH] BeesNote: thread naming fixes

Move pthread_setname_np to the same place we do pthread_getname_np.

Detect errors in pthread_getname_np--but don't throw an exception
because we would call ourself recursively from the exception handler
when it tries to log the exception.

Fix the order of set_name and the first BEESNOTE/BEESLOG call in threads,
closing small time intervals where logs have the wrong thread name,
and that wrong name becomes persistent for the thread.

Make the main thread's name "bees" because Linux kernel stack traces use
the pthread name of the main thread instead of the name of the process.

Anonymous threads get the process name (usually "bees").  We should not
have any such threads, but we do.  This appears to occur mostly during
exception stack unwinding.  GCC/pthread bug?

Fixes:  https://github.com/Zygo/bees/issues/51

Signed-off-by: Zygo Blaxell <bees@furryterror.org>
---
 lib/task.cc        | 16 +++++++++++++++-
 src/bees-thread.cc |  5 +----
 src/bees.cc        | 42 +++++++++++++++++++++++++-----------------
 3 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/lib/task.cc b/lib/task.cc
index abaf9f7..61d1f75 100644
--- a/lib/task.cc
+++ b/lib/task.cc
@@ -85,11 +85,25 @@ namespace crucible {
 	TaskState::exec()
 	{
 		THROW_CHECK0(invalid_argument, m_exec_fn);
+		THROW_CHECK0(invalid_argument, m_print_fn);
+
+		char buf[24];
+		memset(buf, '\0', sizeof(buf));
+		DIE_IF_MINUS_ERRNO(pthread_getname_np(pthread_self(), buf, sizeof(buf)));
+		Cleanup pthread_name_cleaner([&]() {
+			pthread_setname_np(pthread_self(), buf);
+		});
+		ostringstream oss;
+		m_print_fn(oss);
+		auto thread_name = oss.str();
+		DIE_IF_MINUS_ERRNO(pthread_setname_np(pthread_self(), thread_name.c_str()));
+
 		weak_ptr<TaskState> this_task_wp = shared_from_this();
-		Cleanup cleaner([&]() {
+		Cleanup current_task_cleaner([&]() {
 			swap(this_task_wp, tl_current_task_wp);
 		});
 		swap(this_task_wp, tl_current_task_wp);
+
 		m_exec_fn();
 	}
 
diff --git a/src/bees-thread.cc b/src/bees-thread.cc
index de7e4cd..1808ad4 100644
--- a/src/bees-thread.cc
+++ b/src/bees-thread.cc
@@ -15,13 +15,10 @@ BeesThread::exec(function<void()> func)
 	m_timer.reset();
 	BEESLOGDEBUG("BeesThread exec " << m_name);
 	m_thread_ptr = make_shared<thread>([=]() {
-		BEESLOGDEBUG("Starting thread " << m_name);
 		BeesNote::set_name(m_name);
+		BEESLOGDEBUG("Starting thread " << m_name);
 		BEESNOTE("thread function");
 		Timer thread_time;
-		catch_all([&]() {
-			DIE_IF_MINUS_ERRNO(pthread_setname_np(pthread_self(), m_name.c_str()));
-		});
 		catch_all([&]() {
 			func();
 		});
diff --git a/src/bees.cc b/src/bees.cc
index 8056121..c1762bf 100644
--- a/src/bees.cc
+++ b/src/bees.cc
@@ -132,11 +132,15 @@ void
 BeesNote::set_name(const string &name)
 {
 	tl_name = name;
+	catch_all([&]() {
+		DIE_IF_MINUS_ERRNO(pthread_setname_np(pthread_self(), name.c_str()));
+	});
 }
 
 string
 BeesNote::get_name()
 {
+	// Use explicit name if given
 	if (!tl_name.empty()) {
 		return tl_name;
 	}
@@ -145,26 +149,27 @@ BeesNote::get_name()
 	// remember it.  Each output message may be a different Task.
 	// The current task is thread_local so we don't need to worry
 	// about it being destroyed under us.
-	auto current_task = Task::current_task();
-	if (current_task) {
-		ostringstream oss;
-		oss << current_task;
-		return oss.str();
-	}
+        auto current_task = Task::current_task();
+        if (current_task) {
+                ostringstream oss;
+                oss << current_task;
+                return oss.str();
+        }
 
-	// OK try the pthread name next.
-	char buf[100];
+        // OK try the pthread name next.
+	char buf[24];
 	memset(buf, '\0', sizeof(buf));
-	pthread_getname_np(pthread_self(), buf, sizeof(buf));
-	buf[sizeof(buf) - 1] = '\0';
-	tl_name = buf;
-
-	// Give up and use a generic name.
-	if (tl_name.empty()) {
-		tl_name = "bees";
+	int err = pthread_getname_np(pthread_self(), buf, sizeof(buf));
+	if (err) {
+		return string("pthread_getname_np: ") + strerror(err);
 	}
+	buf[sizeof(buf) - 1] = '\0';
 
-	return tl_name;
+	// thread_getname_np returns process name
+	// ...by default?  ...for the main thread?
+	// ...except during exception handling?
+	// ...randomly?
+	return buf;
 }
 
 BeesNote::ThreadStatusMap
@@ -629,8 +634,11 @@ bees_main(int argc, char *argv[])
 		BEESCOUNT(exception_caught);
 	});
 
+	// The thread name for the main function is also what the kernel
+	// Oops messages call the entire process.  So even though this
+	// thread's proper title is "main", let's call it "bees".
+	BeesNote::set_name("bees");
 	BEESNOTE("main");
-	BeesNote::set_name("main");
 
 	list<shared_ptr<BeesContext>> all_contexts;
 	shared_ptr<BeesContext> bc;