From 95f1c03bfe95f77ce1c6a2b342d775c378cab8c8 Mon Sep 17 00:00:00 2001
From: David Garske <david@wolfssl.com>
Date: Thu, 12 Oct 2023 13:39:51 -0700
Subject: [PATCH] Improvements for client property stack. Don't use lock for
 `WOLFMQTT_DYN_PROP`. If using multi-threading make sure
 `clientPropStack_lock` is not de-initialized, since it could be shared by
 multiple client instances. Add CI test. ZD 16814

---
 .github/workflows/ubuntu-check.yml | 11 ++++++++
 src/mqtt_packet.c                  | 41 +++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/.github/workflows/ubuntu-check.yml b/.github/workflows/ubuntu-check.yml
index c0a65c04a..c981b16b4 100644
--- a/.github/workflows/ubuntu-check.yml
+++ b/.github/workflows/ubuntu-check.yml
@@ -76,3 +76,14 @@ jobs:
       if: ${{ failure() && steps.make-check-nonblock-mt.outcome == 'failure' }}
       run: |
         more test-suite.log
+    - name: configure with Multi-threading and WOLFMQTT_DYN_PROP
+      run: ./configure --enable-mt CFLAGS="-DWOLFMQTT_DYN_PROP"
+    - name: make
+      run: make
+    - name: make check
+      id: make-check-mt-dynprop
+      run: make check
+    - name: Show logs on failure
+      if: ${{ failure() && steps.make-check-mt-dynprop.outcome == 'failure' }}
+      run: |
+        more test-suite.log
diff --git a/src/mqtt_packet.c b/src/mqtt_packet.c
index e6d3a7131..0719f98dc 100644
--- a/src/mqtt_packet.c
+++ b/src/mqtt_packet.c
@@ -128,6 +128,8 @@ static const struct MqttPropMatrix gPropMatrix[] = {
 
 /* WOLFMQTT_DYN_PROP allows property allocation using malloc */
 #ifndef WOLFMQTT_DYN_PROP
+
+/* Maximum number of active static properties - overridable */
 #ifndef MQTT_MAX_PROPS
 #define MQTT_MAX_PROPS 30
 #endif
@@ -135,11 +137,12 @@ static const struct MqttPropMatrix gPropMatrix[] = {
 /* Property structure allocation array. Property type equal
    to zero indicates unused element. */
 static MqttProp clientPropStack[MQTT_MAX_PROPS];
-#endif /* WOLFMQTT_DYN_PROP */
 
 #ifdef WOLFMQTT_MULTITHREAD
+static volatile int clientPropStack_lockInit = 0;
 static wm_Sem clientPropStack_lock;
 #endif
+#endif /* WOLFMQTT_DYN_PROP */
 #endif /* WOLFMQTT_V5 */
 
 /* Positive return value is header length, zero or negative indicates error */
@@ -1793,20 +1796,28 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
     return header_len + remain_len;
 }
 
-int MqttProps_Init(void) {
-#ifdef WOLFMQTT_MULTITHREAD
-    return wm_SemInit(&clientPropStack_lock);
-#else
-    return 0;
+int MqttProps_Init(void)
+{
+    int ret = MQTT_CODE_SUCCESS;
+#if !defined(WOLFMQTT_DYN_PROP) && defined(WOLFMQTT_MULTITHREAD)
+    if (clientPropStack_lockInit == 0) {
+        ret = wm_SemInit(&clientPropStack_lock);
+    }
+    clientPropStack_lockInit++;
 #endif
+    return  ret;
 }
 
-int MqttProps_ShutDown(void) {
-#ifdef WOLFMQTT_MULTITHREAD
-    return wm_SemFree(&clientPropStack_lock);
-#else
-    return 0;
+int MqttProps_ShutDown(void)
+{
+    int ret = MQTT_CODE_SUCCESS;
+#if !defined(WOLFMQTT_DYN_PROP) && defined(WOLFMQTT_MULTITHREAD)
+    clientPropStack_lockInit--;
+    if (clientPropStack_lockInit == 0) {
+        ret = wm_SemFree(&clientPropStack_lock);
+    }
 #endif
+    return ret;
 }
 
 /* Add property */
@@ -1821,7 +1832,7 @@ MqttProp* MqttProps_Add(MqttProp **head)
         return NULL;
     }
 
-#ifdef WOLFMQTT_MULTITHREAD
+#if !defined(WOLFMQTT_DYN_PROP) && defined(WOLFMQTT_MULTITHREAD)
     if (wm_SemLock(&clientPropStack_lock) != 0) {
         return NULL;
     }
@@ -1870,7 +1881,7 @@ MqttProp* MqttProps_Add(MqttProp **head)
         (void)MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PROPERTY);
     }
 
-#ifdef WOLFMQTT_MULTITHREAD
+#if !defined(WOLFMQTT_DYN_PROP) && defined(WOLFMQTT_MULTITHREAD)
     (void)wm_SemUnlock(&clientPropStack_lock);
 #endif
 
@@ -1881,7 +1892,7 @@ MqttProp* MqttProps_Add(MqttProp **head)
 int MqttProps_Free(MqttProp *head)
 {
     int ret = MQTT_CODE_SUCCESS;
-#ifdef WOLFMQTT_MULTITHREAD
+#if !defined(WOLFMQTT_DYN_PROP) && defined(WOLFMQTT_MULTITHREAD)
     if ((ret = wm_SemLock(&clientPropStack_lock)) != 0) {
         return ret;
     }
@@ -1898,7 +1909,7 @@ int MqttProps_Free(MqttProp *head)
         head = tmp;
 #endif
     }
-#ifdef WOLFMQTT_MULTITHREAD
+#if !defined(WOLFMQTT_DYN_PROP) && defined(WOLFMQTT_MULTITHREAD)
     (void)wm_SemUnlock(&clientPropStack_lock);
 #endif
     return ret;