From: Tobias Brunner <tobias@strongswan.org>
Date: Tue, 18 Apr 2023 13:46:29 +0000 (+0200)
Subject: watcher: Avoid logging on level 1 while holding the mutex
X-Git-Tag: 5.9.11rc1~27
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7c657e78ffd438d7e2231dec0cba4734eba22536;p=thirdparty%2Fstrongswan.git

watcher: Avoid logging on level 1 while holding the mutex

This could be problematic in case loggers in some way rely on watcher_t
themselves.  This particular log message should rarely occur if at all,
but still avoid holding the mutex.
---

diff --git a/src/libstrongswan/processing/watcher.c b/src/libstrongswan/processing/watcher.c
index 8746f0e3dc..aca36c60fa 100644
--- a/src/libstrongswan/processing/watcher.c
+++ b/src/libstrongswan/processing/watcher.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Tobias Brunner
+ * Copyright (C) 2016-2023 Tobias Brunner
  * Copyright (C) 2013 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -168,20 +168,27 @@ typedef struct {
 } notify_data_t;
 
 /**
- * Notify watcher thread about changes
+ * Notify watcher thread about changes and unlock mutex
  */
-static void update(private_watcher_t *this)
+static void update_and_unlock(private_watcher_t *this)
 {
 	char buf[1] = { 'u' };
+	int error = 0;
 
 	this->pending = TRUE;
 	if (this->notify[1] != -1)
 	{
 		if (write(this->notify[1], buf, sizeof(buf)) == -1)
 		{
-			DBG1(DBG_JOB, "notifying watcher failed: %s", strerror(errno));
+			error = errno;
 		}
 	}
+	this->mutex->unlock(this->mutex);
+
+	if (error)
+	{
+		DBG1(DBG_JOB, "notifying watcher failed: %s", strerror(error));
+	}
 }
 
 /**
@@ -233,9 +240,8 @@ static void notify_end(notify_data_t *data)
 			break;
 		}
 	}
-	update(this);
 	this->condvar->broadcast(this->condvar);
-	this->mutex->unlock(this->mutex);
+	update_and_unlock(this);
 
 	free(data);
 }
@@ -258,7 +264,7 @@ static void notify(private_watcher_t *this, entry_t *entry,
 		.this = this,
 	);
 
-	/* deactivate entry, so we can select() other FDs even if the async
+	/* deactivate entry, so we can poll() other FDs even if the async
 	 * processing did not handle the event yet */
 	entry->in_callback++;
 
@@ -497,15 +503,16 @@ METHOD(watcher_t, add, void,
 	if (this->state == WATCHER_STOPPED)
 	{
 		this->state = WATCHER_QUEUED;
+		this->mutex->unlock(this->mutex);
+
 		lib->processor->queue_job(lib->processor,
 			(job_t*)callback_job_create_with_prio((void*)watch, this,
 				NULL, (callback_job_cancel_t)return_false, JOB_PRIO_CRITICAL));
 	}
 	else
 	{
-		update(this);
+		update_and_unlock(this);
 	}
-	this->mutex->unlock(this->mutex);
 }
 
 METHOD(watcher_t, remove_, void,
@@ -544,9 +551,12 @@ METHOD(watcher_t, remove_, void,
 	}
 	if (found)
 	{
-		update(this);
+		update_and_unlock(this);
+	}
+	else
+	{
+		this->mutex->unlock(this->mutex);
 	}
-	this->mutex->unlock(this->mutex);
 }
 
 METHOD(watcher_t, get_state, watcher_state_t,
diff --git a/src/libstrongswan/processing/watcher.h b/src/libstrongswan/processing/watcher.h
index 3924593194..8a5fababe3 100644
--- a/src/libstrongswan/processing/watcher.h
+++ b/src/libstrongswan/processing/watcher.h
@@ -38,7 +38,7 @@ typedef enum watcher_state_t watcher_state_t;
  * re-enable the event, while the data read can be processed in another
  * asynchronous job.
  *
- * On Linux, even if select() marks an FD as "ready", a subsequent read/write
+ * On Linux, even if poll() marks an FD as "ready", a subsequent read/write
  * can block. It is therefore highly recommended to use non-blocking I/O
  * and handle EAGAIN/EWOULDBLOCK gracefully.
  *
@@ -71,7 +71,7 @@ enum watcher_state_t {
 };
 
 /**
- * Watch multiple file descriptors using select().
+ * Watch multiple file descriptors using poll().
  */
 struct watcher_t {