seqnumber: Fix C++ interopability and mutex cleanup (#1092)

* seqnumber: Fix C++ interopability

_Atomic and std::atomic are not guaranteed to be compatible.
Additionally, std::atomic introduces dependencies on the C++ standard library,
which is often too large for resource-constrained embedded systems.
This change moves all atomic operations into seqnumber.c and uses the C11
atomic_xx operations, which are safe for both C and C++ callers.
It also fixes a bug where _z_seqnumber_null was not implemented as an atomic operation.

* Ensure sequence number is correctly cleaned up in the case of C99 where a mutex is present.

---------

Co-authored-by: Peter van der Perk <[email protected]>

Author: gmartin82

include/zenoh-pico/collections/seqnumber.h

@@ -21,31 +21,23 @@
 #include "zenoh-pico/system/platform.h"
 #include "zenoh-pico/utils/result.h"
 
-#if Z_FEATURE_MULTI_THREAD == 1 && ZENOH_C_STANDARD != 99
-#ifndef __cplusplus
-#include <stdatomic.h>
-#define _Z_SEQNUMBER_TYPE _Atomic uint32_t
-#else
-#include <atomic>
-#define _Z_SEQNUMBER_TYPE std::atomic<uint32_t>
-#endif
-#else
-#define _Z_SEQNUMBER_TYPE uint32_t
-#endif
-
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 typedef struct {
-    _Z_SEQNUMBER_TYPE _seq;
+#if (Z_FEATURE_MULTI_THREAD == 1) && (ZENOH_C_STANDARD != 99) && !defined(__cplusplus)
+    _Atomic uint32_t _seq;
+#else
+    uint32_t _seq;
+#endif
 #if Z_FEATURE_MULTI_THREAD == 1 && ZENOH_C_STANDARD == 99 && !defined(ZENOH_COMPILER_GCC)
     _z_mutex_t _mutex;
 #endif
 } _z_seqnumber_t;
 
-static inline _z_seqnumber_t _z_seqnumber_null(void) { return (_z_seqnumber_t){0}; }
 z_result_t _z_seqnumber_init(_z_seqnumber_t *seq);
+z_result_t _z_seqnumber_drop(_z_seqnumber_t *seq);
 z_result_t _z_seqnumber_fetch(_z_seqnumber_t *seq, uint32_t *value);
 z_result_t _z_seqnumber_fetch_and_increment(_z_seqnumber_t *seq, uint32_t *value);
 

src/api/advanced_publisher.c

@@ -26,13 +26,16 @@
 // Space for 10 digits + NULL
 #define ZE_ADVANCED_PUBLISHER_UINT32_STR_BUF_LEN 11
 
-static _ze_advanced_publisher_state_t _ze_advanced_publisher_state_null(void) {
-    _ze_advanced_publisher_state_t state = {0};
-    state._zn = _z_session_weak_null();
-    z_internal_publisher_null(&state._publisher);
-    state._state_publisher_task_id = _ZP_PERIODIC_SCHEDULER_INVALID_ID;
-    state._seqnumber = _z_seqnumber_null();
-    return state;
+static z_result_t _ze_advanced_publisher_state_init(_ze_advanced_publisher_state_t *state) {
+    if (state == NULL) {
+        _Z_ERROR_RETURN(_Z_ERR_INVALID);
+    }
+    state->_heartbeat_mode = ZE_ADVANCED_PUBLISHER_HEARTBEAT_MODE_NONE;
+    state->_last_published_sn = 0;
+    z_internal_publisher_null(&state->_publisher);
+    state->_state_publisher_task_id = _ZP_PERIODIC_SCHEDULER_INVALID_ID;
+    state->_zn = _z_session_weak_null();
+    return _z_seqnumber_init(&state->_seqnumber);
 }
 
 static bool _ze_advanced_publisher_state_check(const _ze_advanced_publisher_state_t *state) {
@@ -57,7 +60,7 @@ void _ze_advanced_publisher_state_clear(_ze_advanced_publisher_state_t *state) {
     }
     _z_session_weak_drop(&state->_zn);
     state->_heartbeat_mode = ZE_ADVANCED_PUBLISHER_HEARTBEAT_MODE_NONE;
-    state->_seqnumber = _z_seqnumber_null();
+    _z_seqnumber_drop(&state->_seqnumber);
     state->_last_published_sn = 0;
 }
 
@@ -255,18 +258,17 @@ z_result_t ze_declare_advanced_publisher(const z_loaned_session_t *zs, ze_owned_
             z_publisher_drop(z_publisher_move(&pub->_val._publisher));
             _Z_ERROR_RETURN(_Z_ERR_SYSTEM_OUT_OF_MEMORY);
         }
-        *state = _ze_advanced_publisher_state_null();
+        _Z_CLEAN_RETURN_IF_ERR(_ze_advanced_publisher_state_init(state),
+                               z_publisher_drop(z_publisher_move(&pub->_val._publisher));
+                               z_free(state));
 
         pub->_val._state = _ze_advanced_publisher_state_rc_new(state);
         if (_Z_RC_IS_NULL(&pub->_val._state)) {
+            _ze_advanced_publisher_state_clear(state);
             z_free(state);
             z_publisher_drop(z_publisher_move(&pub->_val._publisher));
             _Z_ERROR_RETURN(_Z_ERR_SYSTEM_OUT_OF_MEMORY);
         }
-
-        _Z_CLEAN_RETURN_IF_ERR(_z_seqnumber_init(&state->_seqnumber),
-                               _ze_advanced_publisher_state_rc_drop(&pub->_val._state);
-                               z_publisher_drop(z_publisher_move(&pub->_val._publisher)));
     } else if (opt.cache.is_enabled) {
         pub->_val._sequencing = _ZE_ADVANCED_PUBLISHER_SEQUENCING_TIMESTAMP;
     } else {

src/collections/seqnumber.c

@@ -16,6 +16,10 @@
 
 #include "zenoh-pico/utils/logging.h"
 
+#if Z_FEATURE_MULTI_THREAD == 1 && ZENOH_C_STANDARD != 99
+#include <stdatomic.h>
+#endif
+
 z_result_t _z_seqnumber_init(_z_seqnumber_t *seq) {
     if (seq == NULL) {
         _Z_ERROR_RETURN(_Z_ERR_INVALID);
@@ -28,10 +32,27 @@ z_result_t _z_seqnumber_init(_z_seqnumber_t *seq) {
     }
 #endif
 
+#if (Z_FEATURE_MULTI_THREAD == 1) && (ZENOH_C_STANDARD != 99)
+    atomic_init(&seq->_seq, 0);
+#else
     seq->_seq = 0;
+#endif
+
     return _Z_RES_OK;
 }
 
+z_result_t _z_seqnumber_drop(_z_seqnumber_t *seq) {
+    if (seq == NULL) {
+        _Z_ERROR_RETURN(_Z_ERR_INVALID);
+    }
+
+#if Z_FEATURE_MULTI_THREAD == 1 && ZENOH_C_STANDARD == 99 && !defined(ZENOH_COMPILER_GCC)
+    return _z_mutex_drop(&seq->_mutex);
+#else
+    return _Z_RES_OK;
+#endif
+}
+
 z_result_t _z_seqnumber_fetch(_z_seqnumber_t *seq, uint32_t *value) {
     if (seq == NULL || value == NULL) {
         _Z_ERROR_RETURN(_Z_ERR_INVALID);
@@ -44,7 +65,11 @@ z_result_t _z_seqnumber_fetch(_z_seqnumber_t *seq, uint32_t *value) {
     }
 #endif
 
+#if (Z_FEATURE_MULTI_THREAD == 1) && (ZENOH_C_STANDARD != 99)
+    *value = atomic_load(&seq->_seq);
+#else
     *value = seq->_seq;
+#endif
 
 #if Z_FEATURE_MULTI_THREAD == 1 && ZENOH_C_STANDARD == 99 && !defined(ZENOH_COMPILER_GCC)
     res = _z_mutex_unlock(&seq->_mutex);
@@ -70,6 +95,8 @@ z_result_t _z_seqnumber_fetch_and_increment(_z_seqnumber_t *seq, uint32_t *value
 
 #if Z_FEATURE_MULTI_THREAD == 1 && ZENOH_C_STANDARD == 99 && defined(ZENOH_COMPILER_GCC)
     *value = __sync_fetch_and_add(&seq->_seq, 1);
+#elif (Z_FEATURE_MULTI_THREAD == 1) && (ZENOH_C_STANDARD != 99)
+    *value = atomic_fetch_add(&seq->_seq, 1);
 #else
     *value = seq->_seq++;
 #endif