From a5d51291b687b5c0a599c918f39da5f73ad5a096 Mon Sep 17 00:00:00 2001
From: Tomas Mudrunka <tomas@mudrunka.cz>
Date: Fri, 2 Jul 2021 18:37:55 +0200
Subject: [PATCH] Fixed deadlock in pause enable/disable

---
 c/pthread_extra/pthread_pause.c | 28 ++++++++++++++++++----------
 c/pthread_extra/test_pause.c    | 19 +++++++++++++++++++
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/c/pthread_extra/pthread_pause.c b/c/pthread_extra/pthread_pause.c
index 24521af..f8fb296 100644
--- a/c/pthread_extra/pthread_pause.c
+++ b/c/pthread_extra/pthread_pause.c
@@ -49,9 +49,6 @@ void pthread_pause_handler(const int signal, siginfo_t *info, void *ptr) {
 void pthread_pause_enable() {
 	pthread_pause_init(); //Make sure semaphore is init'd
 
-	//Add thread to internal registry
-	pthread_user_data_internal(pthread_self());
-
 	//Nesting signals too deep is not good for stack
 	//You can get runtime stats using following command:
 	//grep -i sig /proc/$(pgrep binary)/status
@@ -71,17 +68,22 @@ void pthread_pause_enable() {
 		.sa_flags = SA_SIGINFO | SA_RESTART,
 		.sa_restorer = NULL
 	};
+
 	sigaction(PTHREAD_XSIG_STOP, &pause_sa, NULL);
 
 	//Unblock signal
 	pthread_sigmask(SIG_UNBLOCK, &sigset, NULL);
+
+	//Add thread to internal registry
+	pthread_user_data_internal(pthread_self()); //Only now, when signals are unblocked!
 }
 
 void pthread_pause_disable() {
+	//pthread_user_data_lock();
 	pthread_pause_init(); //Make sure semaphore is init'd
 
 	//Add thread to internal registry
-	pthread_user_data_internal(pthread_self());
+	//pthread_user_data_internal(pthread_self()); //DEADLOCKS!
 
 	//Block signal
 	sigset_t sigset;
@@ -89,9 +91,12 @@ void pthread_pause_disable() {
 	sigaddset(&sigset, PTHREAD_XSIG_STOP);
 
 	//Make sure all signals are dispatched before we block them
-	sem_wait(&pthread_pause_sem);
+	//Maybe not a good idea, causes DEADLOCKS!
+	//sem_wait(&pthread_pause_sem);
 	pthread_sigmask(SIG_BLOCK, &sigset, NULL);
-	sem_post(&pthread_pause_sem);
+	//sem_post(&pthread_pause_sem);
+
+	//pthread_user_data_unlock();
 }
 
 /*
@@ -108,7 +113,6 @@ int pthread_pause_reschedule(pthread_t thread) {
 
 int pthread_pause_reschedule(pthread_t thread) {
 	//Decide if the thread should run and signal it
-
 	pthread_user_data_lock();
 
 	//Wait for semaphore which means signal queue is empty
@@ -146,18 +150,20 @@ int pthread_extra_yield() {
 	return pthread_yield();
 }
 
+///Pause specified thread (block until it is paused)
 int pthread_pause(pthread_t thread) {
-	//Set thread as paused and notify it via signal (wait when queue full)
 	pthread_user_data_lock();
+	//Set thread as paused and notify it via signal (wait when queue full)
 	pthread_user_data_internal(thread)->running = 0;
 	pthread_pause_reschedule(thread);
 	pthread_user_data_unlock();
 	return 0;
 }
 
+///UnPause specified thread (block until it is unpaused)
 int pthread_unpause(pthread_t thread) {
-	//Set thread as running and notify it via signal (wait when queue full)
 	pthread_user_data_lock();
+	//Set thread as running and notify it via signal (wait when queue full)
 	pthread_user_data_internal(thread)->running = 1;
 	pthread_pause_reschedule(thread);
 	pthread_user_data_unlock();
@@ -204,8 +210,10 @@ void *pthread_extra_thread_wrapper(void *arg) {
 	pthread_extra_wrapper_t task = *((pthread_extra_wrapper_t*)arg);
 	free(arg);
 
+	pthread_pause_enable();
+
 	//Register new thread to user data structure
-	pthread_user_data_internal(pthread_self()); //Perhaps already done in pthread_extra_yield()??
+	//pthread_user_data_internal(pthread_self()); //Perhaps already done in pthread_extra_yield() and pthread_pause_enable()??
 
 	//TODO: user_data should do this automaticaly?
 	pthread_cleanup_push(pthread_user_data_cleanup, (void *)pthread_self());
diff --git a/c/pthread_extra/test_pause.c b/c/pthread_extra/test_pause.c
index b99c388..c12de5a 100644
--- a/c/pthread_extra/test_pause.c
+++ b/c/pthread_extra/test_pause.c
@@ -1,3 +1,5 @@
+#define PTHREAD_FREEWHEEL
+
 #include <signal.h>
 #include <pthread.h>
 #include <pthread_extra.h>
@@ -6,6 +8,8 @@
 #include <assert.h>
 #include <unistd.h>
 
+//#define printf(...) (0)
+
 pthread_t main_thread;
 
 void *thread_test(void *arg) {
@@ -50,20 +54,35 @@ int main() {
 	while(1) {
 		pthread_pause(b);
 		pthread_unpause(a);
+		pthread_pause_disable();
+		pthread_pause_disable();
+		pthread_pause_disable();
 		printf("SWITCH A:\n");
+		pthread_pause_enable();
+		pthread_pause_enable();
+		pthread_pause_enable();
 		pthread_sleep(2);
 
+		pthread_pause_all();
 		printf("SWITCH B:\n");
+		pthread_unpause_all();
 		pthread_pause(a);
 		pthread_unpause(b);
 		pthread_sleep(2);
 
+		//pthread_pause_disable();
+		pthread_pause_all();
 		printf("SWITCH A+B:\n");
+		pthread_unpause_all();
+		//pthread_pause_enable();
 		pthread_unpause(a);
 		pthread_unpause(b);
 		pthread_sleep(1);
 
+		//pthread_pause_disable();
+		pthread_pause_all();
 		printf("SWITCH MAIN ONLY:\n");
+		//pthread_pause_enable();
 		pthread_pause_all();
 		//printf("\n");
 		pthread_sleep(1);
-- 
2.30.2