Added timeouts and cleanup for zombie callbacks in NON cases.
authorMandeep Shetty <mandeep.shetty@intel.com>
Mon, 27 Apr 2015 23:26:52 +0000 (16:26 -0700)
committerErich Keane <erich.keane@intel.com>
Tue, 28 Apr 2015 22:06:25 +0000 (22:06 +0000)
The client maintains a callback list to pass results upto the app.
If a request, specially a NON request is lost in the network, the
callback will sit in memory forever with no one left to use it.

Added a timeout field for the callback and a default timeout value of 2
hours in ocstackconfig.h. Stack will simultaneously purge timed out callbacks
when going through the callback linked list.

Change-Id: Idbaa7c5c85a40267f101f7bba53c3f98ed29d467
Signed-off-by: Mandeep Shetty <mandeep.shetty@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/843
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Doug Hudson <douglas.hudson@intel.com>
Reviewed-by: Erich Keane <erich.keane@intel.com>
resource/csdk/stack/include/internal/occlientcb.h
resource/csdk/stack/include/ocstackconfig.h
resource/csdk/stack/src/occlientcb.c
resource/csdk/stack/src/ocstack.c

index 05489d6..be296f3 100644 (file)
@@ -69,6 +69,11 @@ typedef struct ClientCB {
     #endif
     // The connectivity type on which the request was sent on.
     OCConnectivityType conType;
+    // The TTL for this callback. Holds the time till when this callback can
+    // still be used. TTL is set to 0 when the callback is for presence and observe.
+    // Presence has ttl mechanism in the "presence" member of this struct and observes
+    // can be explicitly cancelled.
+    uint32_t TTL;
     // next node in this list
     struct ClientCB    *next;
 } ClientCB;
@@ -95,7 +100,8 @@ extern struct ClientCB *cbList;
  *              the resourceType associated with a presence request.
  * @param[in] conType
  *              the connectivity type on which the associated request for this clientCB was sent on.
- *
+ * @param[in] ttl
+ *              time to live in coap_ticks for the callback.
  * @brief If the handle you're looking for does not exist, the stack will reply with a RST message.
  *
  * @return OC_STACK_OK for Success, otherwise some error value
@@ -104,7 +110,7 @@ OCStackResult
 AddClientCB (ClientCB** clientCB, OCCallbackData* cbData,
              CAToken_t token, uint8_t tokenLength,
              OCDoHandle *handle, OCMethod method,
-             char * requestUri, char * resourceTypeName, OCConnectivityType conType);
+             char * requestUri, char * resourceTypeName, OCConnectivityType conType, uint32_t ttl);
 
 /** @ingroup ocstack
  *
index ea6f90f..0213def 100644 (file)
  */
 #define MAX_HEADER_OPTION_DATA_LENGTH (16)
 
+/*
+ * Sets the time to live (TTL) for response callbacks.
+ * The callbacks will be up for deletion after such time but are not guaranteed
+ * to be deleted immediately and you may get responses even after timeout.
+ * This timeout will NOT apply to OBSERVE requests. OBSERVE needs an explicit cancel using OCCancel().
+ * NOTE: Changing the setting to a very long duration may lead to unsupported and untested
+ * operation. Setting this to as small a value as reasonable will reclaim memory faster.
+ */
+#define MAX_CB_TIMEOUT_SECONDS   (2 * 60 * 60)  // 2 hours = 7200 seconds.
+
 #endif //OCSTACK_CONFIG_H_
index a60d444..c2b324f 100644 (file)
 #include "ocmalloc.h"
 #include <string.h>
 
+#ifdef WITH_ARDUINO
+#include "Time.h"
+#else
+#include <sys/time.h>
+#endif
+#include "coap_time.h"
+
 #include "cacommon.h"
 #include "cainterface.h"
 
@@ -38,7 +45,7 @@ OCStackResult
 AddClientCB (ClientCB** clientCB, OCCallbackData* cbData,
              CAToken_t token, uint8_t tokenLength,
              OCDoHandle *handle, OCMethod method,
-             char * requestUri, char * resourceTypeName, OCConnectivityType conType)
+             char * requestUri, char * resourceTypeName, OCConnectivityType conType, uint32_t ttl)
 {
     if(!clientCB || !cbData || !handle || !requestUri || tokenLength > CA_MAX_TOKEN_LEN)
     {
@@ -78,6 +85,17 @@ AddClientCB (ClientCB** clientCB, OCCallbackData* cbData,
             cbNode->presence = NULL;
             cbNode->filterResourceType = NULL;
             #endif // WITH_PRESENCE
+
+            if (method == OC_REST_PRESENCE ||
+                method == OC_REST_OBSERVE  ||
+                method == OC_REST_OBSERVE_ALL)
+            {
+                cbNode->TTL = 0;
+            }
+            else
+            {
+                cbNode->TTL = ttl;
+            }
             cbNode->requestUri = requestUri;
             cbNode->conType = conType;
             LL_APPEND(cbList, cbNode);
@@ -123,7 +141,8 @@ AddClientCB (ClientCB** clientCB, OCCallbackData* cbData,
 
 void DeleteClientCB(ClientCB * cbNode)
 {
-    if(cbNode) {
+    if(cbNode)
+    {
         LL_DELETE(cbList, cbNode);
         OC_LOG(INFO, TAG, PCF("deleting tokens"));
         OC_LOG_BUFFER(INFO, TAG, (const uint8_t *)cbNode->token, cbNode->tokenLength);
@@ -136,7 +155,8 @@ void DeleteClientCB(ClientCB * cbNode)
         }
 
         #ifdef WITH_PRESENCE
-        if(cbNode->presence) {
+        if(cbNode->presence)
+        {
             OCFree(cbNode->presence->timeOut);
             OCFree(cbNode->presence);
         }
@@ -158,25 +178,52 @@ void DeleteClientCB(ClientCB * cbNode)
     }
 }
 
+/*
+ * This function checks if the node is past its time to live and
+ * deletes it if timed-out. Calling this function with a  presence or observe
+ * callback with ttl set to 0 will not delete anything as presence nodes have
+ * their own mechanisms for timeouts. A null argument will cause the function to
+ * silently return.
+ */
+static void CheckAndDeleteTimedOutCB(ClientCB* cbNode)
+{
+    if (!cbNode)
+    {
+        return;
+    }
+    if (cbNode->TTL == 0)
+    {
+        return;
+    }
+    coap_tick_t now;
+    coap_ticks(&now);
+
+    if (cbNode->TTL < now)
+    {
+        OC_LOG(INFO, TAG, PCF("Deleting timed-out callback"));
+        DeleteClientCB(cbNode);
+    }
+}
+
 ClientCB* GetClientCB(const CAToken_t token, uint8_t tokenLength,
         OCDoHandle handle, const char * requestUri)
 {
 
     ClientCB* out = NULL;
 
-    if(token && *token &&
-            tokenLength <= CA_MAX_TOKEN_LEN && tokenLength > 0)
+    if(token && *token && tokenLength <= CA_MAX_TOKEN_LEN && tokenLength > 0)
     {
         LL_FOREACH(cbList, out)
         {
             OC_LOG(INFO, TAG, PCF("comparing tokens"));
             OC_LOG_BUFFER(INFO, TAG, (const uint8_t *)token, tokenLength);
             OC_LOG_BUFFER(INFO, TAG, (const uint8_t *)out->token, tokenLength);
+
             if(memcmp(out->token, token, tokenLength) == 0)
-            if(memcmp(out->token, token, CA_MAX_TOKEN_LEN) == 0)
             {
                 return out;
             }
+            CheckAndDeleteTimedOutCB(out);
         }
     }
     else if(handle)
@@ -187,6 +234,7 @@ ClientCB* GetClientCB(const CAToken_t token, uint8_t tokenLength,
             {
                 return out;
             }
+            CheckAndDeleteTimedOutCB(out);
         }
     }
     else if(requestUri)
@@ -197,6 +245,7 @@ ClientCB* GetClientCB(const CAToken_t token, uint8_t tokenLength,
             {
                 return out;
             }
+            CheckAndDeleteTimedOutCB(out);
         }
     }
     OC_LOG(INFO, TAG, PCF("Callback Not found !!"));
@@ -226,16 +275,19 @@ OCStackResult InsertResourceTypeFilter(ClientCB * cbNode, char * resourceTypeNam
 }
 #endif // WITH_PRESENCE
 
-void DeleteClientCBList() {
+void DeleteClientCBList()
+{
     ClientCB* out;
     ClientCB* tmp;
-    LL_FOREACH_SAFE(cbList, out, tmp) {
+    LL_FOREACH_SAFE(cbList, out, tmp)
+    {
         DeleteClientCB(out);
     }
     cbList = NULL;
 }
 
-void FindAndDeleteClientCB(ClientCB * cbNode) {
+void FindAndDeleteClientCB(ClientCB * cbNode)
+{
     ClientCB* tmp;
     if(cbNode)
     {
@@ -261,7 +313,8 @@ OCStackResult AddMCPresenceNode(OCMulticastNode** outnode, char* uri, uint32_t n
 
     node = (OCMulticastNode*) OCMalloc(sizeof(OCMulticastNode));
 
-    if (node) {
+    if (node)
+    {
         node->nonce = nonce;
         node->uri = uri;
         LL_APPEND(mcPresenceNodes, node);
@@ -272,12 +325,16 @@ OCStackResult AddMCPresenceNode(OCMulticastNode** outnode, char* uri, uint32_t n
     return OC_STACK_NO_MEMORY;
 }
 
-OCMulticastNode* GetMCPresenceNode(const char * uri) {
+OCMulticastNode* GetMCPresenceNode(const char * uri)
+{
     OCMulticastNode* out = NULL;
 
-    if(uri) {
-        LL_FOREACH(mcPresenceNodes, out) {
-            if(out->uri && strcmp(out->uri, uri) == 0) {
+    if(uri)
+    {
+        LL_FOREACH(mcPresenceNodes, out)
+        {
+            if(out->uri && strcmp(out->uri, uri) == 0)
+            {
                 return out;
             }
         }
index b124766..ef609f8 100644 (file)
@@ -1224,6 +1224,12 @@ void HandleCAResponses(const CARemoteEndpoint_t* endPoint, const CAResponseInfo_
                 {
                     FindAndDeleteClientCB(cbNode);
                 }
+                else
+                {
+                    // To keep discovery callbacks active.
+                    cbNode->TTL = GetTicks(MAX_CB_TIMEOUT_SECONDS *
+                                            MILLISECONDS_PER_SECOND);
+                }
             }
 
             //Need to send ACK when the response is CON
@@ -2047,8 +2053,10 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ
         goto exit;
     }
 
-    if((result = AddClientCB(&clientCB, cbData, token, tokenLength,  &resHandle, method,
-                             requestUri, resourceType, conType)) != OC_STACK_OK)
+    result = AddClientCB(&clientCB, cbData, token, tokenLength, &resHandle, method,
+                             requestUri, resourceType, conType,
+                             GetTicks(MAX_CB_TIMEOUT_SECONDS * MILLISECONDS_PER_SECOND));
+    if(result != OC_STACK_OK)
     {
         result = OC_STACK_NO_MEMORY;
         goto exit;