diff --git a/CHANGELOG.md b/CHANGELOG.md index f8f89398..6f570ea1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Changelog for Pelion Device Management Client +### Release 4.11.2 (01.12.2021) + +Fixed a potential dead-lock situation in sn_nsdl.c CoAP tracing when tracing is enabled but trace-level is set below INFO. This fixes a regression introduced in 4.11.0 release. + ### Release 4.11.1 (11.10.2021) ### Device Management Client diff --git a/mbed-client/mbed-client-c/source/sn_nsdl.c b/mbed-client/mbed-client-c/source/sn_nsdl.c index c9b59077..4774b94e 100755 --- a/mbed-client/mbed-client-c/source/sn_nsdl.c +++ b/mbed-client/mbed-client-c/source/sn_nsdl.c @@ -2351,10 +2351,17 @@ bool sn_nsdl_remove_resource_attribute(sn_nsdl_static_resource_parameters_s *par void sn_nsdl_print_coap_data(sn_coap_hdr_s *coap_header_ptr, bool outgoing) { #if MBED_CONF_MBED_TRACE_ENABLE +#if MBED_TRACE_MAX_LEVEL >= TRACE_LEVEL_INFO if (!coap_header_ptr) { return; } + // If active tracing level is below INFO, no need to parse through. + // Use of tr_array() can also result in dead lock with application mutexes if trace level is below INFO. + if (!(mbed_trace_config_get() & TRACE_LEVEL_INFO)) { + return; + } + const char *delimeter = "|"; const char *token = "token:"; const char *payload = "pl:"; @@ -2475,6 +2482,7 @@ void sn_nsdl_print_coap_data(sn_coap_hdr_s *coap_header_ptr, bool outgoing) WRITE_TAG(buffer + ret - 1, buf_size - ret - 1, "%s", end); print_line: + // This is dependant on active-trace level checks earlier in the function. Do not change to debug without adjusting checks for active level. tr_info("%s", buffer); #ifdef MBED_CLIENT_PRINT_COAP_PAYLOAD @@ -2494,11 +2502,11 @@ void sn_nsdl_print_coap_data(sn_coap_hdr_s *coap_header_ptr, bool outgoing) } } #endif - +#endif // MBED_TRACE_MAX_LEVEL #else (void) coap_header_ptr; (void) outgoing; -#endif +#endif // MBED_CONF_MBED_TRACE_ENABLE } #if MBED_CONF_MBED_TRACE_ENABLE diff --git a/mbed-client/mbed-client/m2mresourcebase.h b/mbed-client/mbed-client/m2mresourcebase.h index 1d1447ea..07a9689e 100644 --- a/mbed-client/mbed-client/m2mresourcebase.h +++ b/mbed-client/mbed-client/m2mresourcebase.h @@ -256,7 +256,7 @@ class M2MResourceBase : public M2MBase { * \note If "read_resource_value_callback" is not set this is internally calling value() and value_length() API's. * \param resource Pointer to resource whose value will be read. * \param buffer[OUT] Buffer where the value is stored. - * \param buffer_len[IN/OUT] Buffer size + * \param[in, out] buffer_len On input, tells the maximum size of bytes to read. On output, tells how many bytes have been written to buffer. * \return Error code, 0 on success otherwise < 0 */ int read_resource_value(const M2MResourceBase &resource, void *buffer, size_t *buffer_len); @@ -349,18 +349,21 @@ class M2MResourceBase : public M2MBase { /** * \brief Converts a value to integer and returns it. Note: Conversion * errors are not detected. + * \return int64 value. */ int64_t get_value_int() const; /** * Get the value as a string object. No encoding/charset conversions * are done for the value, just a raw copy. + * \return value as a String object. */ String get_value_string() const; /** * \brief Converts a value to float and returns it. Note: Conversion * errors are not detected. + * \return value as a float. */ float get_value_float() const; @@ -447,6 +450,7 @@ class M2MResourceBase : public M2MBase { * @brief Sets the function that is executed when this * object receives a block-wise message. * @param callback The function pointer that is called. + * @return True if successfully set, otherwise return False. */ bool set_incoming_block_message_callback(incoming_block_message_callback callback); @@ -457,6 +461,7 @@ class M2MResourceBase : public M2MBase { * @note Due to a limitation in the mbed-client-c library, the whole * payload up to 64 KiB must be supplied in the single callback. * @param callback The function pointer that is called. + * @return True if successfully set, otherwise return False. */ bool set_outgoing_block_message_callback(outgoing_block_message_callback callback) m2m_deprecated; diff --git a/mbed-client/mbed-client/m2mversion.h b/mbed-client/mbed-client/m2mversion.h index 6c2f8f4d..afe5a8e6 100644 --- a/mbed-client/mbed-client/m2mversion.h +++ b/mbed-client/mbed-client/m2mversion.h @@ -30,6 +30,6 @@ /** PDMC_PATCH_VERSION * Pelion Device Management Client patch version */ -#define PDMC_PATCH_VERSION 1 +#define PDMC_PATCH_VERSION 2 #endif // M2MVERSION_H diff --git a/mbed-client/source/include/m2minterfaceimpl.h b/mbed-client/source/include/m2minterfaceimpl.h index 80a98699..51f8dd85 100644 --- a/mbed-client/source/include/m2minterfaceimpl.h +++ b/mbed-client/source/include/m2minterfaceimpl.h @@ -351,7 +351,7 @@ class M2MInterfaceImpl : public M2MInterface, virtual void registration_updated(const M2MServer &server_object); - virtual void registration_error(uint8_t error_code, bool retry = false, bool full_registration = false); + virtual void registration_error(uint8_t error_code, bool retry = false, bool full_registration = false, bool ping_recovery = false); virtual void client_unregistered(bool success = true); diff --git a/mbed-client/source/include/m2mnsdlobserver.h b/mbed-client/source/include/m2mnsdlobserver.h index ddb2b50c..cf43dadf 100644 --- a/mbed-client/source/include/m2mnsdlobserver.h +++ b/mbed-client/source/include/m2mnsdlobserver.h @@ -61,8 +61,9 @@ public : * @param error_code, Error code for registration error * @param retry, Indicates state machine to re-establish connection * @param full_registration, Indicates that after DTLS handshake continue with a full registration + * @param ping_recovery, Indicates whether ping recovery should be attempted or not */ - virtual void registration_error(uint8_t error_code, bool retry = false, bool full_registration = false) = 0; + virtual void registration_error(uint8_t error_code, bool retry = false, bool full_registration = false, bool ping_recovery = false) = 0; /** * @brief Informs that client is unregistered. diff --git a/mbed-client/source/m2minterfaceimpl.cpp b/mbed-client/source/m2minterfaceimpl.cpp index 54366af8..4eb0a0d9 100644 --- a/mbed-client/source/m2minterfaceimpl.cpp +++ b/mbed-client/source/m2minterfaceimpl.cpp @@ -401,12 +401,12 @@ void M2MInterfaceImpl::registration_updated(const M2MServer &server_object) _observer.registration_updated(_register_server, server_object); } -void M2MInterfaceImpl::registration_error(uint8_t error_code, bool retry, bool full_registration) +void M2MInterfaceImpl::registration_error(uint8_t error_code, bool retry, bool full_registration, bool ping_recovery) { tr_error("M2MInterfaceImpl::registration_error code [%d]", error_code); if (_binding_mode == M2MInterface::UDP || _binding_mode == M2MInterface::UDP_QUEUE) { - if (error_code != M2MInterface::MemoryFail && _connection_handler.is_cid_available()) { + if (ping_recovery && _connection_handler.is_cid_available()) { // Check if we can ping LWm2m server with DTLS client hello (send it immediately and lets have timeout of 60 seconds) // if(server responds) // CID has expired, delete CID do handshake diff --git a/mbed-client/source/m2mnsdlinterface.cpp b/mbed-client/source/m2mnsdlinterface.cpp index 46034efb..1f582ce2 100644 --- a/mbed-client/source/m2mnsdlinterface.cpp +++ b/mbed-client/source/m2mnsdlinterface.cpp @@ -899,7 +899,7 @@ uint8_t M2MNsdlInterface::received_from_server_callback(struct nsdl_s *nsdl_hand free_response_list(); } - _observer.registration_error(M2MInterface::NetworkError, true); + _observer.registration_error(M2MInterface::NetworkError, true, false, true); // Handle Server-side expections during registration flow // Client might receive error from server due to temporary connection/operability reasons, @@ -917,7 +917,7 @@ uint8_t M2MNsdlInterface::received_from_server_callback(struct nsdl_s *nsdl_hand tr_error("M2MNsdlInterface::received_from_server_callback - registration error %d", coap_header->msg_code); tr_error("M2MNsdlInterface::received_from_server_callback - unexpected error received from server"); // Try to do clean register again - _observer.registration_error(M2MInterface::NetworkError, true); + _observer.registration_error(M2MInterface::NetworkError, true, false, false); } else { // Add warn for any message that gets this far. We might be missing some handling in above. @@ -1374,7 +1374,7 @@ void M2MNsdlInterface::timer_expired(M2MTimerObserver::Type type) tr_debug("M2MNsdlInterface::timer_expired - Send update registration"); if (!send_update_registration()) { // Most likely case would be memory allocation failure - _observer.registration_error(M2MInterface::MemoryFail, false); + _observer.registration_error(M2MInterface::MemoryFail, false, false, false); } } else if (M2MTimerObserver::RetryTimer == type) { send_pending_request(); @@ -3296,17 +3296,18 @@ void M2MNsdlInterface::handle_register_response(const sn_coap_hdr_s *coap_header } else { tr_error("M2MNsdlInterface::handle_register_response - registration error %d", coap_header->msg_code); - if (coap_header->coap_status == COAP_STATUS_BUILDER_MESSAGE_SENDING_FAILED || - coap_header->coap_status == COAP_STATUS_BUILDER_BLOCK_SENDING_FAILED) { - tr_error("M2MNsdlInterface::handle_register_response - message sending failed !!!!"); - } if (COAP_MSG_CODE_RESPONSE_BAD_REQUEST == coap_header->msg_code || COAP_MSG_CODE_RESPONSE_FORBIDDEN == coap_header->msg_code) { - _observer.registration_error(M2MInterface::InvalidParameters, false); + _observer.registration_error(M2MInterface::InvalidParameters, false, false, false); + } else if (coap_header->coap_status == COAP_STATUS_BUILDER_MESSAGE_SENDING_FAILED || + coap_header->coap_status == COAP_STATUS_BUILDER_BLOCK_SENDING_FAILED) { + tr_error("M2MNsdlInterface::handle_register_response - message sending failed !!!!"); + _observer.registration_error(M2MInterface::NetworkError, true, true, true); } else { + tr_error("M2MNsdlInterface::handle_register_response - Try to do clean register again"); // Try to do clean register again - _observer.registration_error(M2MInterface::NetworkError, true, true); + _observer.registration_error(M2MInterface::NetworkError, true, true, false); } } } @@ -3350,7 +3351,7 @@ void M2MNsdlInterface::handle_register_update_response(const sn_coap_hdr_s *coap coap_header->coap_status == COAP_STATUS_BUILDER_BLOCK_SENDING_FAILED) { // Inform interfaceimpl to do a reconnection and registration update // till we get CoAP level response for the request - _observer.registration_error(M2MInterface::NetworkError, true); + _observer.registration_error(M2MInterface::NetworkError, true, false, true); } else { // Clear observation tokens and do a full registration send_next_notification(M2MNsdlInterface::CLEAR_NOTIFICATION_TOKEN); @@ -3442,7 +3443,7 @@ void M2MNsdlInterface::handle_request_response(const sn_coap_hdr_s *coap_header, // Retransmission completed if (coap_header->coap_status == COAP_STATUS_BUILDER_MESSAGE_SENDING_FAILED || coap_header->coap_status == COAP_STATUS_BUILDER_BLOCK_SENDING_FAILED) { - _observer.registration_error(M2MInterface::NetworkError, true); + _observer.registration_error(M2MInterface::NetworkError, true, false, true); // Start retry logic, only for file download operation } else if (coap_header->msg_code == COAP_MSG_CODE_RESPONSE_SERVICE_UNAVAILABLE && @@ -3540,7 +3541,7 @@ bool M2MNsdlInterface::handle_post_response(sn_coap_hdr_s *coap_header, tr_debug("M2MNsdlInterface::handle_post_response - Send Update registration for Create"); if (!send_update_registration()) { // Most likely case would be memory allocation failure - _observer.registration_error(M2MInterface::MemoryFail, false); + _observer.registration_error(M2MInterface::MemoryFail, false, false, false); } } } else {