From 13b3dfc922d841b808902ef37fb3f9129f109083 Mon Sep 17 00:00:00 2001 From: Jon Shallow Date: Fri, 7 Feb 2025 10:39:29 +0000 Subject: [PATCH] KeepAlive: Better session cleanup after KeepAlive failure --- examples/coap-client.c | 2 +- include/coap3/coap_session_internal.h | 10 +++++++++ include/coap3/coap_subscribe_internal.h | 12 ++++++++++ src/coap_io.c | 11 +--------- src/coap_net.c | 12 +++++++--- src/coap_resource.c | 25 ++++++++++++--------- src/coap_session.c | 29 +++++++++++++++++++++++++ 7 files changed, 77 insertions(+), 24 deletions(-) diff --git a/examples/coap-client.c b/examples/coap-client.c index 03199e17b2..5430d1354d 100644 --- a/examples/coap-client.c +++ b/examples/coap-client.c @@ -2101,7 +2101,7 @@ main(int argc, char **argv) { obs_ms -= result; } } - if (ready && repeat_count) { + if ((ready || obs_ms) && repeat_count) { /* Send off next request if appropriate */ if (repeat_ms > (unsigned)result) { repeat_ms -= (unsigned)result; diff --git a/include/coap3/coap_session_internal.h b/include/coap3/coap_session_internal.h index f284864c75..e5519ece30 100644 --- a/include/coap3/coap_session_internal.h +++ b/include/coap3/coap_session_internal.h @@ -604,6 +604,16 @@ coap_session_t *coap_new_client_session_psk2_lkd(coap_context_t *ctx, coap_session_t *coap_session_new_dtls_session(coap_session_t *session, coap_tick_t now); +/** + * Clear down a session following a keepalive failure. + * The event handler will get notified of the failure. + * Note: the @p session cannot be used after this function is called. + * + * @param session Session to clear down. + * + */ +void coap_session_server_keepalive_failed(coap_session_t *session); + void coap_session_free(coap_session_t *session); void coap_session_mfree(coap_session_t *session); diff --git a/include/coap3/coap_subscribe_internal.h b/include/coap3/coap_subscribe_internal.h index c9cceed6f4..68a2372e23 100644 --- a/include/coap3/coap_subscribe_internal.h +++ b/include/coap3/coap_subscribe_internal.h @@ -167,6 +167,18 @@ int coap_delete_observer_request(coap_resource_t *resource, */ void coap_delete_observers(coap_context_t *context, coap_session_t *session); +/** + * Removes specific subscription for @p session for @p resource and releases + * the allocated storage. + * + * @param resource The resource + * @param session The observer's session. + * @param subscription The observer's subscription. + */ +void coap_delete_observer_internal(coap_resource_t *resource, + coap_session_t *session, + coap_subscription_t *subscription); + /** * Initiate the sending of an Observe packet for all observers of @p resource, * optionally matching @p query if not NULL diff --git a/src/coap_io.c b/src/coap_io.c index e940ec9954..62a3889337 100644 --- a/src/coap_io.c +++ b/src/coap_io.c @@ -1490,16 +1490,7 @@ coap_io_prepare_io_lkd(coap_context_t *ctx, /* Some issue - not safe to continue processing */ continue; if (s->last_ping > 0 && s->last_pong < s->last_ping) { - coap_handle_event_lkd(s->context, COAP_EVENT_KEEPALIVE_FAILURE, s); - coap_session_reference_lkd(s); - RESOURCES_ITER(s->context->resources, r) { - coap_cancel_all_messages(s->context, s, NULL); - coap_delete_observer(r, s, NULL); - } - coap_session_release_lkd(s); - /* Force session to go away */ - coap_session_set_type_client_lkd(s); - coap_session_release_lkd(s); + coap_session_server_keepalive_failed(s); /* check the next session */ continue; } diff --git a/src/coap_net.c b/src/coap_net.c index 816b65c111..815196ca58 100644 --- a/src/coap_net.c +++ b/src/coap_net.c @@ -763,9 +763,9 @@ coap_free_context_lkd(coap_context_t *context) { #endif /* COAP_SERVER_SUPPORT */ coap_delete_all(context->sendqueue); + context->sendqueue = NULL; #ifdef WITH_LWIP - context->sendqueue = NULL; if (context->timer_configured) { LOCK_TCPIP_CORE(); sys_untimeout(coap_io_process_timeout, (void *)context); @@ -2167,8 +2167,14 @@ coap_retransmit(coap_context_t *context, coap_queue_t *node) { #if COAP_SERVER_SUPPORT /* Check if subscriptions exist that should be canceled after COAP_OBS_MAX_FAIL */ - if (COAP_RESPONSE_CLASS(node->pdu->code) >= 2) { - coap_handle_failed_notify(context, node->session, &node->pdu->actual_token); + if (COAP_RESPONSE_CLASS(node->pdu->code) >= 2 && node->session->ref_subscriptions) { + if (context->ping_timeout) { + coap_session_server_keepalive_failed(node->session); + coap_delete_node_lkd(node); + return COAP_INVALID_MID; + } else { + coap_handle_failed_notify(context, node->session, &node->pdu->actual_token); + } } #endif /* COAP_SERVER_SUPPORT */ if (node->session->con_active) { diff --git a/src/coap_resource.c b/src/coap_resource.c index 24401bb1da..c3543f3f06 100644 --- a/src/coap_resource.c +++ b/src/coap_resource.c @@ -513,13 +513,7 @@ coap_free_resource(coap_resource_t *resource) { /* free all elements from resource->subscribers */ LL_FOREACH_SAFE(resource->subscribers, obs, otmp) { - if (resource->context->observe_deleted) - resource->context->observe_deleted(obs->session, obs, - resource->context->observe_user_data); - coap_session_release_lkd(obs->session); - coap_delete_pdu_lkd(obs->pdu); - coap_delete_cache_key(obs->cache_key); - coap_free_type(COAP_SUBSCRIPTION, obs); + coap_delete_observer_internal(resource, obs->session, obs); } if (resource->proxy_name_count && resource->proxy_name_list) { size_t i; @@ -996,7 +990,7 @@ coap_touch_observer(coap_context_t *context, coap_session_t *session, } } -static void +void coap_delete_observer_internal(coap_resource_t *resource, coap_session_t *session, coap_subscription_t *s) { if (!s) @@ -1005,6 +999,8 @@ coap_delete_observer_internal(coap_resource_t *resource, coap_session_t *session if (coap_get_log_level() >= COAP_LOG_DEBUG) { char outbuf[2 * 8 + 1] = ""; unsigned int i; + coap_string_t *uri_path; + coap_string_t *uri_query; for (i = 0; i < s->pdu->actual_token.length; i++) { size_t size = strlen(outbuf); @@ -1012,9 +1008,18 @@ coap_delete_observer_internal(coap_resource_t *resource, coap_session_t *session snprintf(&outbuf[size], sizeof(outbuf)-size, "%02x", s->pdu->actual_token.s[i]); } - coap_log_debug("removed subscription %p with token '%s' key 0x%02x%02x%02x%02x\n", + uri_path = coap_get_uri_path(s->pdu); + uri_query = coap_get_query(s->pdu); + coap_log_debug("removed subscription '/%*.*s%s%*.*s' (%p) with token '%s' key 0x%02x%02x%02x%02x\n", + uri_path ? (int)uri_path->length : 0, uri_path ? (int)uri_path->length : 0, + uri_path ? (char *)uri_path->s : "", + uri_query ? "?" : "", + uri_query ? (int)uri_query->length : 0, uri_query ? (int)uri_query->length : 0, + uri_query ? (char *)uri_query->s : "", (void *)s, outbuf, s->cache_key->key[0], s->cache_key->key[1], s->cache_key->key[2], s-> cache_key->key[3]); + coap_delete_string(uri_path); + coap_delete_string(uri_query); } if (session->context->observe_deleted) session->context->observe_deleted(session, s, @@ -1022,9 +1027,9 @@ coap_delete_observer_internal(coap_resource_t *resource, coap_session_t *session if (resource->subscribers) { LL_DELETE(resource->subscribers, s); - coap_session_release_lkd(session); assert(session->ref_subscriptions > 0); session->ref_subscriptions--; + coap_session_release_lkd(session); coap_delete_pdu_lkd(s->pdu); coap_delete_cache_key(s->cache_key); coap_free_type(COAP_SUBSCRIPTION, s); diff --git a/src/coap_session.c b/src/coap_session.c index 207416e4c4..986bbec8c2 100644 --- a/src/coap_session.c +++ b/src/coap_session.c @@ -601,6 +601,35 @@ coap_session_free(coap_session_t *session) { coap_free_type(COAP_SESSION, session); } +#if COAP_SERVER_SUPPORT +void +coap_session_server_keepalive_failed(coap_session_t *session) { + coap_session_reference_lkd(session); + coap_handle_event_lkd(session->context, COAP_EVENT_KEEPALIVE_FAILURE, session); + coap_cancel_all_messages(session->context, session, NULL); + RESOURCES_ITER(session->context->resources, r) { + int i; + + /* In case code is broken somewhere */ + for (i = 0; i < 1000; i++) { + if (!coap_delete_observer(r, session, NULL)) + break; + } + } + while (session->delayqueue) { + coap_queue_t *q = session->delayqueue; + + session->delayqueue = q->next; + coap_delete_node_lkd(q); + } + /* Force session to go away */ + coap_session_set_type_client_lkd(session); + coap_session_release_lkd(session); + + coap_session_release_lkd(session); +} +#endif /* COAP_SERVER_SUPPORT */ + static size_t coap_session_max_pdu_size_internal(const coap_session_t *session, size_t max_with_header) {