Skip to content

Commit

Permalink
Improvements for client property stack. Don't use lock for `WOLFMQTT_…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
dgarske committed Oct 20, 2023
1 parent d8e9699 commit 0afb4e3
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 15 deletions.
11 changes: 11 additions & 0 deletions .github/workflows/ubuntu-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,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
41 changes: 26 additions & 15 deletions src/mqtt_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,21 @@ 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

/* 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 */
Expand Down Expand Up @@ -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) {
clientPropStack_lockInit++;
ret = wm_SemInit(&clientPropStack_lock);
}
#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 */
Expand All @@ -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;
}
Expand Down Expand Up @@ -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

Expand All @@ -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;
}
Expand All @@ -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;
Expand Down

0 comments on commit 0afb4e3

Please sign in to comment.