pkgsrc-WIP-changes archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

btop-git: prevent stalls when running for long times.



Module Name:	pkgsrc-wip
Committed By:	Santhosh Raju <fox%NetBSD.org@localhost>
Pushed By:	fox
Date:		Sun May 10 09:41:14 2026 +0200
Changeset:	f01cd62698da69001ca5492c681438a2a99a3674

Modified Files:
	btop-git/distinfo
	btop-git/patches/patch-src_btop.cpp
Removed Files:
	btop-git/TODO

Log Message:
btop-git: prevent stalls when running for long times.

To see a diff of this commit:
https://wip.pkgsrc.org/cgi-bin/gitweb.cgi?p=pkgsrc-wip.git;a=commitdiff;h=f01cd62698da69001ca5492c681438a2a99a3674

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

diffstat:
 btop-git/TODO                       |   2 -
 btop-git/distinfo                   |   2 +-
 btop-git/patches/patch-src_btop.cpp | 150 +++++++++++++++++++++---------------
 3 files changed, 91 insertions(+), 63 deletions(-)

diffs:
diff --git a/btop-git/TODO b/btop-git/TODO
deleted file mode 100644
index 9704034175..0000000000
--- a/btop-git/TODO
+++ /dev/null
@@ -1,2 +0,0 @@
-- Check on CPU graph render when there are 16 or more cores.
-
diff --git a/btop-git/distinfo b/btop-git/distinfo
index e2c26259bf..eacd9fcc25 100644
--- a/btop-git/distinfo
+++ b/btop-git/distinfo
@@ -4,4 +4,4 @@ BLAKE2s (btop-1.4.7-6e39144aaf5a6bc01b9f795010b0914431067183.tar.gz) = 7a1d8fea4
 SHA512 (btop-1.4.7-6e39144aaf5a6bc01b9f795010b0914431067183.tar.gz) = b90d96cc53bde84637535584af13109f6998d2f5a0be4143829814ff30ad4da71173822a5aa4c96a790a86a7950258197504350c17567ed8a330eca3c24bc1bc
 Size (btop-1.4.7-6e39144aaf5a6bc01b9f795010b0914431067183.tar.gz) = 1280353 bytes
 SHA1 (patch-Makefile) = 7cec0d616dd445ad139f0868141d15d1687bf402
-SHA1 (patch-src_btop.cpp) = 46edbcad224ba31643bf14b5942bb195e3ecdfeb
+SHA1 (patch-src_btop.cpp) = d82f33f0f894d2ecfe622116b650ef06dac42666
diff --git a/btop-git/patches/patch-src_btop.cpp b/btop-git/patches/patch-src_btop.cpp
index 2057b6688a..c8151dd156 100644
--- a/btop-git/patches/patch-src_btop.cpp
+++ b/btop-git/patches/patch-src_btop.cpp
@@ -1,75 +1,105 @@
 $NetBSD$
 
-Attempt to fix UI hangs when running for long periods
-
-https://github.com/aristocratos/btop/pull/1647
+Fix pthread_cancel deadlocks on NetBSD:
+1. Use POSIX sem_t instead of std::binary_semaphore (C++ semaphore internal
+   waiter pool is not cancellation-safe)
+2. Add pthread_cleanup_push/pop for Runner::mtx to ensure it's released
+   when thread is cancelled
+3. Wake thread after pthread_cancel by posting to semaphore, since NetBSD's
+   sem_wait doesn't automatically wake on cancellation
+4. Check for cancellation in semaphore acquire loop
 
 --- src/btop.cpp.orig	2026-05-06 15:50:33.000000000 +0000
 +++ src/btop.cpp
-@@ -127,6 +127,7 @@ namespace Global {
+@@ -44,6 +44,8 @@ tab-size = 4
+ #include <chrono>
+ #include <utility>
+ #include <semaphore>
++#include <semaphore.h>  // POSIX semaphore - cancellation-safe unlike std::binary_semaphore
++#include <cerrno>
  
- namespace Runner {
- 	static pthread_t runner_id;
-+	void safe_thread_trigger();
- } // namespace Runner
+ #ifdef __APPLE__
+ 	#include <CoreFoundation/CoreFoundation.h>
+@@ -370,8 +372,21 @@ namespace Runner {
+ 	}
  
- //* Handler for SIGWINCH and general resizing events, does nothing if terminal hasn't been resized unless force=true
-@@ -215,6 +216,12 @@ void clean_quit(int sig) {
- 	Runner::stop();
- 	if (Global::_runner_started) {
- 	#if defined __APPLE__ || defined __OpenBSD__ || defined __NetBSD__
-+		// binary_semaphore::acquire() is not a POSIX cancellation point on macOS;
-+		// pthread_join() would hang if the thread is parked in thread_wait(). Post
-+		// a token so it wakes, sees Global::quitting==true, and exits the loop.
-+		// safe_thread_trigger() drains any pre-existing token to avoid undefined behavior on the
-+		// max-1 semaphore.
-+		Runner::safe_thread_trigger();
- 		if (pthread_join(Runner::runner_id, nullptr) != 0) {
- 			Logger::warning("Failed to join _runner thread on exit!");
- 			pthread_cancel(Runner::runner_id);
-@@ -374,6 +381,9 @@ namespace Runner {
- 	std::binary_semaphore do_work { 0 };
+ 	//* Setup semaphore for triggering thread to do work
+-	// TODO: This can be made a local without too much effort.
+-	std::binary_semaphore do_work { 0 };
++	//* Using POSIX sem_t instead of std::binary_semaphore because the C++ semaphore
++	//* is NOT cancellation-safe - pthread_cancel can corrupt its internal waiter pool
++	struct posix_semaphore {
++		sem_t sem;
++		posix_semaphore() { sem_init(&sem, 0, 0); }
++		~posix_semaphore() { sem_destroy(&sem); }
++		void acquire() {
++			while (sem_wait(&sem) == -1 && errno == EINTR) {
++				pthread_testcancel();  // Check for pending cancellation
++			}
++			pthread_testcancel();  // Check after successful acquire too
++		}
++		void release() { sem_post(&sem); }
++	};
++	posix_semaphore do_work;
  	inline void thread_wait() { do_work.acquire(); }
  	inline void thread_trigger() { do_work.release(); }
-+	// Guarantees exactly one token is posted: drains any pre-existing token
-+	// before release() to avoid undefined behavior on the max-1 binary_semaphore.
-+	inline void safe_thread_trigger() { do_work.try_acquire(); do_work.release(); }
  
- 	//* Wrapper for raising privileges when using SUID bit
- 	class gain_priv {
-@@ -742,7 +752,13 @@ namespace Runner {
- 			Logger::error("Stall in Runner thread, restarting!");
- 			set_active(false);
+@@ -458,6 +473,12 @@ namespace Runner {
+ 		}
+ 	}
+ 
++	//? Cleanup handler to release mutex when thread is cancelled via pthread_cancel
++	static void cleanup_mutex(void* arg) {
++		std::mutex* m = static_cast<std::mutex*>(arg);
++		m->unlock();
++	}
++
+ 	//? ------------------------------- Secondary thread: async launcher and drawing ----------------------------------
+ 	static void * _runner(void *) {
+ 		//? Block some signals in this thread to avoid deadlock from any signal handlers trying to stop this thread
+@@ -468,8 +489,10 @@ namespace Runner {
+ 		sigaddset(&mask, SIGTERM);
+ 		pthread_sigmask(SIG_BLOCK, &mask, nullptr);
+ 
+-		// TODO: On first glance it looks redudant with `Runner::active`.
+-		std::lock_guard lock {mtx};
++		//? Lock mutex and register cleanup handler to release it on thread cancellation
++		//? This prevents deadlock when pthread_cancel is called on a stalled thread
++		mtx.lock();
++		pthread_cleanup_push(cleanup_mutex, &mtx);
+ 
+ 		//* ----------------------------------------------- THREAD LOOP -----------------------------------------------
+ 		while (not Global::quitting) {
+@@ -725,12 +748,22 @@ namespace Runner {
+ 
+ 			//? If overlay isn't empty, print output without color and then print overlay on top
+ 			const bool term_sync = Config::getB("terminal_sync");
++			// Disable cancellation so pthread_cancel() cannot interrupt between
++			// sync_start and sync_end — an incomplete synchronized frame leaves
++			// the terminal holding a stale
++			// image indefinitely (observed on macOS Terminal.app).
++			int old_cancel_state = PTHREAD_CANCEL_ENABLE;
++			pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancel_state);
+ 			cout << (term_sync ? Term::sync_start : "") << (conf.overlay.empty()
+ 					? output
+ 					: (output.empty() ? "" : Fx::ub + Theme::c("inactive_fg") + Fx::uncolor(output)) + conf.overlay)
+ 				<< (term_sync ? Term::sync_end : "") << flush;
++			pthread_setcancelstate(old_cancel_state, nullptr);
+ 		}
+ 		//* ----------------------------------------------- THREAD LOOP -----------------------------------------------
++
++		//? Pop cleanup handler and execute it (unlocks the mutex)
++		pthread_cleanup_pop(1);
+ 		return {};
+ 	}
+ 	//? ------------------------------------------ Secondary thread end -----------------------------------------------
+@@ -744,6 +777,9 @@ namespace Runner {
  			// exit(1);
-+			// pthread_cancel() marks cancellation pending before thread_trigger() wakes
-+			// the thread. binary_semaphore::acquire() is not a POSIX cancellation point
-+			// on macOS; sleep_ms(1) is. With cancellation already pending when the
-+			// thread reaches sleep_ms(), delivery is guaranteed regardless of scheduling.
-+			stopping = true;
  			pthread_cancel(Runner::runner_id);
-+			thread_trigger();
  
++			//? Wake thread from sem_wait so it can process the cancellation
++			thread_trigger();
++
  			// Wait for the thread to actually terminate before creating a new one
  			void* thread_result;
-@@ -750,6 +766,21 @@ namespace Runner {
- 			if (join_result != 0) {
- 				Logger::warning("Failed to join cancelled thread: {}", strerror(join_result));
- 			}
-+			// pthread_cancel does not reliably invoke C++ destructors on macOS,
-+			// so the cancelled thread's lock_guard on mtx may not have fired.
-+			// The thread is gone (joined above); unlock the orphaned mutex so
-+			// the replacement thread can acquire it. Technically undefined
-+			// behavior per [thread.mutex.requirements.mutex] (calling unlock
-+			// from a non-owning thread), but the mutex would otherwise remain
-+			// permanently locked.
-+			if (join_result == 0) {
-+				mtx.unlock();
-+			}
-+			// A thread cancelled mid-work never calls do_work.acquire(), leaving the
-+			// counter at 1. Drain it so the new thread blocks on its first thread_wait()
-+			// rather than racing on current_conf.
-+			do_work.try_acquire();
-+			stopping = false;
- 
- 			if (pthread_create(&Runner::runner_id, nullptr, &Runner::_runner, nullptr) != 0) {
- 				Global::exit_error_msg = "Failed to re-create _runner thread!";
+ 			int join_result = pthread_join(Runner::runner_id, &thread_result);


Home | Main Index | Thread Index | Old Index