Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constrained Stack: No need for individual locks #1481

Merged
merged 1 commit into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions include/coap3/coap_mutex_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_ */
19 changes: 2 additions & 17 deletions src/coap_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 1 addition & 11 deletions src/coap_mbedtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand All @@ -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);
Expand Down
64 changes: 2 additions & 62 deletions src/coap_net.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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 */
Expand All @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
Expand All @@ -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 */
Expand Down Expand Up @@ -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();
}

Expand Down
12 changes: 1 addition & 11 deletions src/coap_subscribe.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down