]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
watcher: Avoid logging on level 1 while holding the mutex
authorTobias Brunner <tobias@strongswan.org>
Tue, 18 Apr 2023 13:46:29 +0000 (15:46 +0200)
committerTobias Brunner <tobias@strongswan.org>
Thu, 27 Apr 2023 11:45:32 +0000 (13:45 +0200)
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.

src/libstrongswan/processing/watcher.c
src/libstrongswan/processing/watcher.h

index 8746f0e3dccb7aa455618df5831d89eb1958e8aa..aca36c60fa8ac00541a2a1f32fe8498278b499a3 100644 (file)
@@ -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,
index 3924593194be0b67eb2011e9d2408a8f58b9cfe2..8a5fababe38f5609ed7ecdb8c11d7a233b514b20 100644 (file)
@@ -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 {