From 0cbcaa6c0d92f73bb1bc7d2be247c33fe52f9981 Mon Sep 17 00:00:00 2001
From: Jon Shallow <supjps-libcoap@jpshallow.com>
Date: Wed, 13 Mar 2024 15:45:10 +0000
Subject: [PATCH] Coverity: Fix 1584343

Reorganize code in the coap_lock_lock_func().
---
 include/coap3/coap_mutex_internal.h |  2 +-
 src/coap_threadsafe.c               | 41 ++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/include/coap3/coap_mutex_internal.h b/include/coap3/coap_mutex_internal.h
index cf403ff9bd..4318e2999b 100644
--- a/include/coap3/coap_mutex_internal.h
+++ b/include/coap3/coap_mutex_internal.h
@@ -219,7 +219,7 @@ typedef struct coap_lock_t {
   unsigned int callback_line;
   unsigned int being_freed;
   unsigned int in_callback;
-  volatile unsigned int lock_count;
+  unsigned int lock_count;
 } coap_lock_t;
 
 void coap_lock_unlock_func(coap_lock_t *lock, const char *file, int line);
diff --git a/src/coap_threadsafe.c b/src/coap_threadsafe.c
index d721dff579..7d65af8a31 100644
--- a/src/coap_threadsafe.c
+++ b/src/coap_threadsafe.c
@@ -776,27 +776,32 @@ coap_lock_unlock_func(coap_lock_t *lock, const char *file, int line) {
 
 int
 coap_lock_lock_func(coap_lock_t *lock, const char *file, int line) {
-  if (lock->in_callback && coap_thread_pid == lock->pid) {
-    lock->lock_count++;
-    assert(lock->in_callback == lock->lock_count);
-  } else {
-    if (coap_mutex_trylock(&lock->mutex)) {
-      if (coap_thread_pid == lock->pid) {
+  if (coap_mutex_trylock(&lock->mutex)) {
+    if (coap_thread_pid == lock->pid) {
+      /* This thread locked the mutex */
+      if (lock->in_callback) {
+        /* This is called from within an app callback */
+        lock->lock_count++;
+        assert(lock->in_callback == lock->lock_count);
+        return 1;
+      } else {
         coap_log_alert("Thread Deadlock: Last %s: %u, this %s: %u\n",
                        lock->lock_file, lock->lock_line, file, line);
         assert(0);
       }
-      coap_mutex_lock(&lock->mutex);
-    }
-    /* Just got the lock, so should not be in a locked callback */
-    assert(!lock->in_callback);
-    lock->pid = coap_thread_pid;
-    lock->lock_file = file;
-    lock->lock_line = line;
-    if (lock->being_freed) {
-      coap_lock_unlock_func(lock, file, line);
-      return 0;
     }
+    /* Wait for the other thread to unlock */
+    coap_mutex_lock(&lock->mutex);
+  }
+  /* Just got the lock, so should not be in a locked callback */
+  assert(!lock->in_callback);
+  lock->pid = coap_thread_pid;
+  lock->lock_file = file;
+  lock->lock_line = line;
+  if (lock->being_freed) {
+    /* context is in the process of being deleted */
+    coap_lock_unlock_func(lock, file, line);
+    return 0;
   }
   return 1;
 }
@@ -817,6 +822,10 @@ coap_lock_unlock_func(coap_lock_t *lock) {
 
 int
 coap_lock_lock_func(coap_lock_t *lock) {
+  /*
+   * Some OS do not have support for coap_mutex_trylock() so
+   * cannot use that here and have to rely on lock-pid being stable
+   */
   if (lock->in_callback && coap_thread_pid == lock->pid) {
     lock->lock_count++;
     assert(lock->in_callback == lock->lock_count);