Merge pull request #97 from PelionIoT/release-4.11.2

mbed-cloud-client 4.11.2

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

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

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;
 

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

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);
 

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.

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

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 {