Test_Trace_Flusher.flush_async_flushes_on_flusher_thread fails: std::strlen(new_name_cstr) <= max_thread_name_length

[herrhotzenplotz]

This is on FreeBSD 14.0-RELEASE-p4:

[ RUN      ] Test_Trace_Flusher.flush_async_flushes_on_flusher_thread
/usr/home/nico/src/quick-lint-js/src/quick-lint-js/port/thread-name.cpp:59: internal check failed in set_current_thread_name: std::strlen(new_name_cstr) <= max_thread_name_length
quick-lint-js crashed. Please report this bug here:
https://quick-lint-js.com/crash-report/
Process 5675 stopped
* thread #3, name = 'quick-lint-js-te', stop reason = signal SIGILL: privileged instruction
    frame #0: 0x0000000000e75804 quick-lint-js-test`quick_lint_js::set_current_thread_name(new_name="quick-lint-js Trace_Flusher") at thread-name.cpp:59:3
   56  	#endif
   57  	
   58  	#if defined(__FreeBSD__)
-> 59  	QLJS_ASSERT(std::strlen(new_name_cstr) <= max_thread_name_length);
   60  	int rc = ::pthread_setname_np(::pthread_self(), new_name_cstr);
   61  	if (rc != 0) {
   62  	  QLJS_DEBUG_LOG(
(lldb) up
frame #1: 0x0000000000e23326 quick-lint-js-test`quick_lint_js::Trace_Flusher::start_flushing_thread()::$_0::operator()(this=0x000003336f6c53a0) const at trace-flusher.cpp:195:7
   192 	  if constexpr (max_thread_name_length < 16) {
   193 	    set_current_thread_name(u8"qljs-tracing");
   194 	  } else {
-> 195 	    set_current_thread_name(u8"quick-lint-js Trace_Flusher");
   196 	  }
   197 	  Lock_Ptr<Shared_State> state = this->state_.lock();
   198 	  while (!state->stop_flushing_thread) {
(lldb) down
frame #0: 0x0000000000e75804 quick-lint-js-test`quick_lint_js::set_current_thread_name(new_name="quick-lint-js Trace_Flusher") at thread-name.cpp:59:3
   56  	#endif
   57  	
   58  	#if defined(__FreeBSD__)
-> 59  	QLJS_ASSERT(std::strlen(new_name_cstr) <= max_thread_name_length);
   60  	int rc = ::pthread_setname_np(::pthread_self(), new_name_cstr);
   61  	if (rc != 0) {
   62  	  QLJS_DEBUG_LOG(
(lldb) p strlen(new_name_cstr)
(size_t) $0 = 27
(lldb) that's not 16 bytes dude!^C
(lldb) q
Quitting LLDB will kill one or more processes. Do you really want to proceed: [Y/n] 
$

Obviously the bug is that the constexpr if checks for a different length than the one of the actual string being passed to set_current_thread_name.

The following patch fixes the issue:

diff --git a/src/quick-lint-js/logging/trace-flusher.cpp b/src/quick-lint-js/logging/trace-flusher.cpp
--- a/src/quick-lint-js/logging/trace-flusher.cpp
+++ b/src/quick-lint-js/logging/trace-flusher.cpp
@@ -189,10 +189,11 @@ void Trace_Flusher::flush_async() { this
 void Trace_Flusher::start_flushing_thread() {
   QLJS_ASSERT(!this->flushing_thread_.joinable());
   this->flushing_thread_.start([this]() {
-    if constexpr (max_thread_name_length < 16) {
+    constexpr Char8 long_thread_name[] = u8"quick-lint-js Trace_Flusher";
+    if constexpr (max_thread_name_length < sizeof(long_thread_name) - 1) {
       set_current_thread_name(u8"qljs-tracing");
     } else {
-      set_current_thread_name(u8"quick-lint-js Trace_Flusher");
+      set_current_thread_name(long_thread_name);
     }
     Lock_Ptr<Shared_State> state = this->state_.lock();
     while (!state->stop_flushing_thread) {

[strager]

Thanks for reporting.

Background_Thread_Pipe_Writer::run_flushing_thread has the same bug (src/quick-lint-js/io/pipe-writer.cpp line 87). (However, that code only runs on Windows right now, and Windows has a huge thread name limit.)

I'll make a more robust solution.

[strager]

Fixed in Git commit 4a6f1fc (in the master branch).