WIP: mostly works, but sometimes deadlocks

mguetschow · mguetschow · commit 92e63e50c089 · 2025-10-24T16:00:04.000+02:00
diff --git a/pkg/tinydtls/contrib/sock_dtls.c b/pkg/tinydtls/contrib/sock_dtls.c
@@ -31,7 +31,7 @@
 #include "net/sock/async/event.h"
 #endif
 
-#define ENABLE_DEBUG 0
+#define ENABLE_DEBUG 1
 #include "debug.h"
 #include "dtls_debug.h"
 
@@ -104,7 +104,8 @@ static int _read(struct dtls_context_t *ctx, session_t *session, uint8_t *buf, s
 #ifdef SOCK_HAS_ASYNC
     if (sock->async_cb != NULL) {
         /* reset retrievable event session */
-        memset(&sock->async_cb_session, 0, sizeof(sock->async_cb_session));
+        // todo: why would we need to do that? if message comes to fast, CONN_RDY might not have been processed yet
+        // memset(&sock->async_cb_session, 0, sizeof(sock->async_cb_session));
         sock->async_cb(sock, SOCK_ASYNC_MSG_RECV, sock->async_cb_arg);
     }
 #endif
@@ -143,7 +144,8 @@ static int _event(struct dtls_context_t *ctx, session_t *session,
         }
     }
     if (!level && (code != DTLS_EVENT_CONNECT)) {
-        mbox_put(&sock->mbox, &msg);
+        // todo: do we even need mbox for async?
+        mbox_try_put(&sock->mbox, &msg);
     }
 
 #if IS_ACTIVE(CONFIG_DTLS_ECC)
@@ -165,11 +167,17 @@ static int _event(struct dtls_context_t *ctx, session_t *session,
             memcpy(&sock->async_cb_session, session, sizeof(session_t));
             sock->async_cb(sock, SOCK_ASYNC_CONN_FIN, sock->async_cb_arg);
             break;
-        case DTLS_EVENT_CONNECTED:
+        case DTLS_EVENT_CONNECT:
             /* we received a session handshake initialization */
             sock->async_cb(sock, SOCK_ASYNC_CONN_RECV,
                            sock->async_cb_arg);
             break;
+        case DTLS_EVENT_CONNECTED:
+            /* session handshake complete */
+            memcpy(&sock->async_cb_session, session, sizeof(session_t));
+            sock->async_cb(sock, SOCK_ASYNC_CONN_RDY,
+                           sock->async_cb_arg);
+            break;
         default:
             break;
         }
@@ -724,7 +732,7 @@ ssize_t sock_dtls_sendv_aux(sock_dtls_t *sock, sock_dtls_session_t *remote,
 #if SOCK_HAS_ASYNC
 /**
  * @brief   Checks for and iterates for more data chunks within the network
- *          stacks anternal packet buffer
+ *          stacks internal packet buffer
  *
  * When no more chunks exists, `data_ctx` assures cleaning up the internal
  * buffer state and `sock_udp_recv_buf()` returns 0.
@@ -1048,7 +1056,8 @@ void _udp_cb(sock_udp_t *udp_sock, sock_async_flags_t flags, void *ctx)
             return;
         }
         _ep_to_session(&remote_ep, &remote);
-        sock->buf_ctx = data_ctx;
+        // todo: why would we even need sock->buf_ctx??
+        // sock->buf_ctx = data_ctx;
         res = dtls_handle_message(sock->dtls_ctx, &remote,
                                   data, res);
         if (sock->buffer.data == NULL) {
diff --git a/sys/net/application_layer/unicoap/drivers/rfc7252/dtls/transport.c b/sys/net/application_layer/unicoap/drivers/rfc7252/dtls/transport.c
@@ -62,16 +62,33 @@ static void _dtls_on_event(sock_dtls_t* sock, sock_async_flags_t type, void* arg
     (void)arg;
     sock_dtls_session_t session = { 0 };
 
-    if (type & SOCK_ASYNC_CONN_RECV) {
-        DTLS_DEBUG("establishing session\n");
-        ssize_t res = sock_dtls_recv(sock, &session, unicoap_receiver_buffer,
-                                     sizeof(unicoap_receiver_buffer),
-                                     CONFIG_UNICOAP_DTLS_HANDSHAKE_TIMEOUT_MS * US_PER_MS);
-
-        if (res != -SOCK_DTLS_HANDSHAKE) {
-            DTLS_DEBUG("could not establish DTLS session: %" PRIiSIZE " (%s)\n", res,
-                       strerror(-(int)res));
-            goto error;
+    // if (type & SOCK_ASYNC_CONN_RECV) {
+    //     DTLS_DEBUG("establishing session...\n");
+    //     ssize_t res = sock_dtls_recv(sock, &session, unicoap_receiver_buffer,
+    //                                  sizeof(unicoap_receiver_buffer),
+    //                                  CONFIG_UNICOAP_DTLS_HANDSHAKE_TIMEOUT_MS * US_PER_MS);
+    //     // _UNICOAP_DEBUG_HEX(unicoap_receiver_buffer, res);
+
+    //     if (-res != SOCK_DTLS_HANDSHAKE) {
+    //         DTLS_DEBUG("could not establish DTLS session: %" PRIiSIZE " (%s)\n", res,
+    //                    strerror(-(int)res));
+    //         goto error;
+    //     }
+    // }
+
+    DTLS_DEBUG("CONN_RDY = %d, MSG_RECV = %d, CONN_FIN = %d, CONN_RECV = %d\n",
+        (type & SOCK_ASYNC_CONN_RDY) != 0,
+        (type & SOCK_ASYNC_MSG_RECV) != 0,
+        (type & SOCK_ASYNC_CONN_FIN) != 0,
+        (type & SOCK_ASYNC_CONN_RECV) != 0);
+
+    if (type & SOCK_ASYNC_CONN_RDY) {
+        DTLS_DEBUG("session established\n");
+
+        if (!sock_dtls_get_event_session(sock, &session)) {
+            DTLS_DEBUG("session was established, but the corresponding session "
+                       "could not be retrieved from the socket\n");
+            return;
         }
 
         dsm_state_t prev_state = dsm_store(sock, &session, SESSION_STATE_ESTABLISHED, false);
@@ -82,7 +99,7 @@ static void _dtls_on_event(sock_dtls_t* sock, sock_async_flags_t type, void* arg
          * waiting thread to inform about established session */
         if (prev_state == SESSION_STATE_HANDSHAKE) {
             msg_t msg = { .type = DTLS_EVENT_CONNECTED };
-            msg_send(&msg, _dtls_auth_waiting_thread);
+            msg_try_send(&msg, _dtls_auth_waiting_thread);
         }
         else if (prev_state == NO_SPACE) {
             /* No space in session management. Should not happen. If it occurs,
@@ -102,40 +119,17 @@ static void _dtls_on_event(sock_dtls_t* sock, sock_async_flags_t type, void* arg
         }
     }
 
-    if (type & SOCK_ASYNC_CONN_FIN) {
-        DTLS_DEBUG("closing session\n");
-        if (sock_dtls_get_event_session(sock, &session)) {
-            /* Session is already destroyed, only remove it from session mgmt. */
-            dsm_remove(sock, &session);
-        }
-        else {
-            DTLS_DEBUG("session was closed, but the corresponding session "
-                       "could not be retrieved from the socket\n");
-            return;
-        }
-
-        DTLS_DEBUG("session ended, removing associated endpoint state\n");
-
-        unicoap_endpoint_t endpoint = { .proto = UNICOAP_PROTO_DTLS };
-        sock_dtls_session_get_udp_ep(&session, unicoap_endpoint_get_dtls(&endpoint));
-        unicoap_exchange_release_endpoint_state(&endpoint);
-        /* It is safe to ignore the result of exchange_release_endpoint state as this logic follows
-         * a best-effort philosophy. */
-    }
-
-    if (type & SOCK_ASYNC_CONN_RDY) {
-        DTLS_DEBUG("connection ready\n");
-    }
-
     if (type & SOCK_ASYNC_MSG_RECV) {
         DTLS_DEBUG("received encrypted datagram\n");
+
         sock_dtls_aux_rx_t aux_rx = {
             .flags = IS_ACTIVE(CONFIG_UNICOAP_GET_LOCAL_ENDPOINTS) ? SOCK_AUX_GET_LOCAL : 0,
         };
 
         void* pdu = NULL;
         void* buffer_ctx = NULL;
 
+        // todo: this should be called again with return value 0, but `buffer_ctx` is apparently ignored in sock-async case
         ssize_t received = sock_dtls_recv_buf_aux(sock, &session, &pdu, &buffer_ctx, 0, &aux_rx);
         if (received < 0) {
             DTLS_DEBUG("recv failure: %" PRIdSIZE "\n", received);
@@ -146,6 +140,12 @@ static void _dtls_on_event(sock_dtls_t* sock, sock_async_flags_t type, void* arg
         if (received == 0) {
             return;
         }
+        // void *empty_pdu = NULL;
+        // ssize_t empty_received = sock_dtls_recv_buf_aux(sock, &session, &empty_pdu, &buffer_ctx, , &aux_rx);
+        // if (empty_received != 0) {
+        //     DTLS_DEBUG("failure or unexpected second buf in recv: %" PRIdSIZE "\n", empty_received);
+        //     return;
+        // }
 
         assert(pdu);
 
@@ -167,6 +167,24 @@ static void _dtls_on_event(sock_dtls_t* sock, sock_async_flags_t type, void* arg
         unicoap_messaging_process_rfc7252((uint8_t*)pdu, received, false, &packet);
     }
 
+    if (type & SOCK_ASYNC_CONN_FIN) {
+        if (sock_dtls_get_event_session(sock, &session)) {
+            /* Session is already destroyed, only remove it from dsm */
+            dsm_remove(sock, &session);
+        }
+        else {
+            DTLS_DEBUG("session was closed, but the corresponding session "
+                       "could not be retrieved from the socket\n");
+            return;
+        }
+
+        DTLS_DEBUG("session ended, removing associated endpoint state\n");
+
+        unicoap_endpoint_t endpoint = { .proto = UNICOAP_PROTO_DTLS };
+        sock_dtls_session_get_udp_ep(&session, unicoap_endpoint_get_dtls(&endpoint));
+        unicoap_exchange_release_endpoint_state(&endpoint);
+    }
+
     return;
 
 error:
@@ -236,12 +254,13 @@ int unicoap_transport_sendv_dtls(iolist_t* iolist, const sock_udp_ep_t* remote,
             return res;
         }
     }
-    /* prepare session */
-    sock_dtls_session_set_udp_ep(session, remote);
-    dsm_state_t session_state = dsm_store(&_dtls_socket, session, SESSION_STATE_HANDSHAKE, true);
-    if (session_state == NO_SPACE) {
-        return -1;
-    }
+    // todo: not needed, already done in _dtls_authenticate, or session is already established?!
+    // /* prepare session */
+    // sock_dtls_session_set_udp_ep(session, remote);
+    // dsm_state_t session_state = dsm_store(&_dtls_socket, session, SESSION_STATE_HANDSHAKE, true);
+    // if (session_state == NO_SPACE) {
+    //     return -1;
+    // }
 
     DTLS_DEBUG("started sending\n");
 
