Constrained Stack: No need for individual locks

If a proxy reads in a packet via coap_read_endpoint, which then
invokes an ongoing proxy TCP connection, while waiting for the CSM
response of the new session, coap_read_endpoint would get invoked
again if there is any data to read on any of the listening endpoints..

With a Constrained Stack, the read in data is protected by mutex
m_read_enpoint - which is still locked when trying to wait for the
CSM.

As the code is either single threaded, or protected by global_lock
if multi-threaded, there is no need to maintain mutexes such as
m_read_endpoint.  This PR removes these not needed mutexes.

include/coap3/coap_mutex_internal.h

@@ -150,15 +150,4 @@ typedef int coap_mutex_t;
 
 #endif /* !WITH_CONTIKI && !WITH_LWIP && !RIOT_VERSION && !HAVE_PTHREAD_H && !HAVE_PTHREAD_MUTEX_LOCK */
 
-#if COAP_CONSTRAINED_STACK
-
-extern coap_mutex_t m_show_pdu;
-extern coap_mutex_t m_log_impl;
-extern coap_mutex_t m_dtls_recv;
-extern coap_mutex_t m_read_session;
-extern coap_mutex_t m_read_endpoint;
-extern coap_mutex_t m_persist_add;
-
-#endif /* COAP_CONSTRAINED_STACK */
-
 #endif /* COAP_MUTEX_INTERNAL_H_ */

src/coap_debug.c

@@ -778,7 +778,7 @@ void
 coap_show_pdu(coap_log_t level, const coap_pdu_t *pdu) {
 #if COAP_CONSTRAINED_STACK
   /* Proxy-Uri: can be 1034 bytes long */
-  /* buf and outbuf protected by mutex m_show_pdu */
+  /* buf and outbuf can be protected by global_lock if needed */
   static unsigned char buf[min(COAP_DEBUG_BUF_SIZE, 1035)];
   static char outbuf[COAP_DEBUG_BUF_SIZE];
 #else /* ! COAP_CONSTRAINED_STACK */
@@ -803,10 +803,6 @@ coap_show_pdu(coap_log_t level, const coap_pdu_t *pdu) {
   if (level > coap_get_log_level())
     return;
 
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_lock(&m_show_pdu);
-#endif /* COAP_CONSTRAINED_STACK */
-
   if (!pdu->session || COAP_PROTO_NOT_RELIABLE(pdu->session->proto)) {
     snprintf(outbuf, sizeof(outbuf), "v:%d t:%s c:%s i:%04x {",
              COAP_DEFAULT_VERSION, msg_type_string(pdu->type),
@@ -1118,10 +1114,6 @@ coap_show_pdu(coap_log_t level, const coap_pdu_t *pdu) {
     outbuflen--;
   snprintf(&outbuf[outbuflen], sizeof(outbuf)-outbuflen,  "\n");
   COAP_DO_SHOW_OUTPUT_LINE;
-
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_unlock(&m_show_pdu);
-#endif /* COAP_CONSTRAINED_STACK */
 }
 
 void
@@ -1273,28 +1265,21 @@ coap_log_impl(coap_log_t level, const char *format, ...) {
 
   if (log_handler) {
 #if COAP_CONSTRAINED_STACK
-    /* message protected by mutex m_log_impl */
+    /* message can be protected by global_lock if needed */
     static char message[COAP_DEBUG_BUF_SIZE];
 #else /* ! COAP_CONSTRAINED_STACK */
     char message[COAP_DEBUG_BUF_SIZE];
 #endif /* ! COAP_CONSTRAINED_STACK */
     va_list ap;
     va_start(ap, format);
 
-#if COAP_CONSTRAINED_STACK
-    coap_mutex_lock(&m_log_impl);
-#endif /* COAP_CONSTRAINED_STACK */
-
 #ifdef RIOT_VERSION
     flash_vsnprintf(message, sizeof(message), format, ap);
 #else /* !RIOT_VERSION */
     vsnprintf(message, sizeof(message), format, ap);
 #endif /* !RIOT_VERSION */
     va_end(ap);
     log_handler(level, message);
-#if COAP_CONSTRAINED_STACK
-    coap_mutex_unlock(&m_log_impl);
-#endif /* COAP_CONSTRAINED_STACK */
   } else {
     char timebuf[32];
     coap_tick_t now;

src/coap_mbedtls.c

@@ -2299,16 +2299,12 @@ coap_dtls_receive(coap_session_t *c_session,
 
   if (m_env->established) {
 #if COAP_CONSTRAINED_STACK
-    /* pdu protected by mutex m_dtls_recv */
+    /* pdu can be protected by global_lock if needed */
     static uint8_t pdu[COAP_RXBUFFER_SIZE];
 #else /* ! COAP_CONSTRAINED_STACK */
     uint8_t pdu[COAP_RXBUFFER_SIZE];
 #endif /* ! COAP_CONSTRAINED_STACK */
 
-#if COAP_CONSTRAINED_STACK
-    coap_mutex_lock(&m_dtls_recv);
-#endif /* COAP_CONSTRAINED_STACK */
-
     if (c_session->state == COAP_SESSION_STATE_HANDSHAKE) {
       coap_handle_event_lkd(c_session->context, COAP_EVENT_DTLS_CONNECTED,
                             c_session);
@@ -2318,9 +2314,6 @@ coap_dtls_receive(coap_session_t *c_session,
     ret = mbedtls_ssl_read(&m_env->ssl, pdu, sizeof(pdu));
     if (ret > 0) {
       ret = coap_handle_dgram(c_session->context, c_session, pdu, (size_t)ret);
-#if COAP_CONSTRAINED_STACK
-      coap_mutex_unlock(&m_dtls_recv);
-#endif /* COAP_CONSTRAINED_STACK */
       goto finish;
     }
     switch (ret) {
@@ -2337,9 +2330,6 @@ coap_dtls_receive(coap_session_t *c_session,
                     -ret, get_error_string(ret), data_len);
       break;
     }
-#if COAP_CONSTRAINED_STACK
-    coap_mutex_unlock(&m_dtls_recv);
-#endif /* COAP_CONSTRAINED_STACK */
     ret = -1;
   } else {
     ret = do_mbedtls_handshake(c_session, m_env);

src/coap_net.c

@@ -2002,7 +2002,7 @@ coap_write_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now
 void
 coap_read_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now) {
 #if COAP_CONSTRAINED_STACK
-  /* payload and packet protected by mutex m_read_session */
+  /* payload and packet can be protected by global_lock if needed */
   static unsigned char payload[COAP_RXBUFFER_SIZE];
   static coap_packet_t s_packet;
 #else /* ! COAP_CONSTRAINED_STACK */
@@ -2011,10 +2011,6 @@ coap_read_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now)
 #endif /* ! COAP_CONSTRAINED_STACK */
   coap_packet_t *packet = &s_packet;
 
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_lock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
-
   assert(session->sock.flags & (COAP_SOCKET_CONNECTED | COAP_SOCKET_MULTICAST));
 
   packet->length = sizeof(payload);
@@ -2052,25 +2048,16 @@ coap_read_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now)
       /* Need max space incase PDU is updated with updated token etc. */
       pdu = coap_pdu_init(0, 0, 0, coap_session_max_pdu_rcv_size(session));
       if (!pdu) {
-#if COAP_CONSTRAINED_STACK
-        coap_mutex_unlock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
         return;
       }
 
       if (!coap_pdu_parse(session->proto, packet->payload, bytes_read, pdu)) {
         coap_handle_event_lkd(session->context, COAP_EVENT_BAD_PACKET, session);
         coap_log_warn("discard malformed PDU\n");
         coap_delete_pdu(pdu);
-#if COAP_CONSTRAINED_STACK
-        coap_mutex_unlock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
         return;
       }
 
-#if COAP_CONSTRAINED_STACK
-      coap_mutex_unlock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
       coap_dispatch(ctx, session, pdu);
       coap_delete_pdu(pdu);
       return;
@@ -2102,13 +2089,7 @@ coap_read_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now)
           if (n == len) {
             if (coap_pdu_parse_header(session->partial_pdu, session->proto)
                 && coap_pdu_parse_opt(session->partial_pdu)) {
-#if COAP_CONSTRAINED_STACK
-              coap_mutex_unlock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
               coap_dispatch(ctx, session, session->partial_pdu);
-#if COAP_CONSTRAINED_STACK
-              coap_mutex_lock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
             }
             coap_delete_pdu(session->partial_pdu);
             session->partial_pdu = NULL;
@@ -2154,13 +2135,7 @@ coap_read_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now)
             session->partial_read = hdr_size + tok_ext_bytes;
             if (size == 0) {
               if (coap_pdu_parse_header(session->partial_pdu, session->proto)) {
-#if COAP_CONSTRAINED_STACK
-                coap_mutex_unlock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
                 coap_dispatch(ctx, session, session->partial_pdu);
-#if COAP_CONSTRAINED_STACK
-                coap_mutex_lock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
               }
               coap_delete_pdu(session->partial_pdu);
               session->partial_pdu = NULL;
@@ -2185,9 +2160,6 @@ coap_read_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now)
       coap_session_disconnected_lkd(session, COAP_NACK_NOT_DELIVERABLE);
 #endif /* !COAP_DISABLE_TCP */
   }
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_unlock(&m_read_session);
-#endif /* COAP_CONSTRAINED_STACK */
 }
 
 #if COAP_SERVER_SUPPORT
@@ -2196,7 +2168,7 @@ coap_read_endpoint(coap_context_t *ctx, coap_endpoint_t *endpoint, coap_tick_t n
   ssize_t bytes_read = -1;
   int result = -1;                /* the value to be returned */
 #if COAP_CONSTRAINED_STACK
-  /* payload and e_packet protected by mutex m_read_endpoint */
+  /* payload and e_packet can be protected by global_lock if needed */
   static unsigned char payload[COAP_RXBUFFER_SIZE];
   static coap_packet_t e_packet;
 #else /* ! COAP_CONSTRAINED_STACK */
@@ -2208,10 +2180,6 @@ coap_read_endpoint(coap_context_t *ctx, coap_endpoint_t *endpoint, coap_tick_t n
   assert(COAP_PROTO_NOT_RELIABLE(endpoint->proto));
   assert(endpoint->sock.flags & COAP_SOCKET_BOUND);
 
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_lock(&m_read_endpoint);
-#endif /* COAP_CONSTRAINED_STACK */
-
   /* Need to do this as there may be holes in addr_info */
   memset(&packet->addr_info, 0, sizeof(packet->addr_info));
   packet->length = sizeof(payload);
@@ -2234,9 +2202,6 @@ coap_read_endpoint(coap_context_t *ctx, coap_endpoint_t *endpoint, coap_tick_t n
         coap_session_new_dtls_session(session, now);
     }
   }
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_unlock(&m_read_endpoint);
-#endif /* COAP_CONSTRAINED_STACK */
   return result;
 }
 
@@ -4392,14 +4357,6 @@ coap_check_async(coap_context_t *context, coap_tick_t now) {
 
 int coap_started = 0;
 
-#if COAP_CONSTRAINED_STACK
-coap_mutex_t m_show_pdu;
-coap_mutex_t m_log_impl;
-coap_mutex_t m_dtls_recv;
-coap_mutex_t m_read_session;
-coap_mutex_t m_read_endpoint;
-coap_mutex_t m_persist_add;
-#endif /* COAP_CONSTRAINED_STACK */
 #if COAP_THREAD_SAFE
 /*
  * Global lock for multi-thread support
@@ -4418,15 +4375,6 @@ coap_startup(void) {
     return;
   coap_started = 1;
 
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_init(&m_show_pdu);
-  coap_mutex_init(&m_log_impl);
-  coap_mutex_init(&m_dtls_recv);
-  coap_mutex_init(&m_read_session);
-  coap_mutex_init(&m_read_endpoint);
-  coap_mutex_init(&m_persist_add);
-#endif /* COAP_CONSTRAINED_STACK */
-
 #if COAP_THREAD_SAFE
   coap_lock_init();
 #endif /* COAP_THREAD_SAFE */
@@ -4476,14 +4424,6 @@ coap_cleanup(void) {
 #endif /* WITH_LWIP */
   coap_dtls_shutdown();
 
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_destroy(&m_show_pdu);
-  coap_mutex_destroy(&m_log_impl);
-  coap_mutex_destroy(&m_dtls_recv);
-  coap_mutex_destroy(&m_read_session);
-  coap_mutex_destroy(&m_read_endpoint);
-  coap_mutex_destroy(&m_persist_add);
-#endif /* COAP_CONSTRAINED_STACK */
   coap_debug_reset();
 }
 

src/coap_subscribe.c

@@ -77,7 +77,7 @@ coap_persist_observe_add_lkd(coap_context_t *context,
   size_t data_len;
   coap_pdu_t *pdu = NULL;
 #if COAP_CONSTRAINED_STACK
-  /* e_packet protected by mutex m_persist_add */
+  /* e_packet can be protected by global_lock if needed */
   static coap_packet_t e_packet;
 #else /* ! COAP_CONSTRAINED_STACK */
   coap_packet_t e_packet;
@@ -110,10 +110,6 @@ coap_persist_observe_add_lkd(coap_context_t *context,
   if (!ep)
     return NULL;
 
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_lock(&m_persist_add);
-#endif /* COAP_CONSTRAINED_STACK */
-
   /* Build up packet */
   memcpy(&packet->addr_info, s_addr_info, sizeof(packet->addr_info));
   packet->ifindex = 0;
@@ -301,17 +297,11 @@ coap_persist_observe_add_lkd(coap_context_t *context,
   (void)oscore_info;
 #endif /* ! COAP_OSCORE_SUPPORT */
   coap_delete_pdu(pdu);
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_unlock(&m_persist_add);
-#endif /* COAP_CONSTRAINED_STACK */
   return s;
 
 malformed:
   coap_log_warn("coap_persist_observe_add: discard malformed PDU\n");
 fail:
-#if COAP_CONSTRAINED_STACK
-  coap_mutex_unlock(&m_persist_add);
-#endif /* COAP_CONSTRAINED_STACK */
   coap_delete_string(uri_path);
   coap_delete_pdu(pdu);
   return NULL;