From 989fcf480b12b46cf08b8d06d57a888d847882bb Mon Sep 17 00:00:00 2001
From: Iain Anderson <38766128+iain-anderson@users.noreply.github.com>
Date: Thu, 14 Oct 2021 16:12:31 +0100
Subject: [PATCH] fix: stop device autoevents before releasing memory (#382)

refac: eliminate recursion from dup/free list functions

Signed-off-by: Iain Anderson <iain@iotechsys.com>
---
 scripts/Dockerfile.alpine-base |  1 -
 src/c/autoevent.c              | 27 ++++------
 src/c/autoevent.h              |  1 -
 src/c/devmap.c                 | 41 ++++++++++-----
 src/c/edgex-rest.c             | 94 ++++++++++++++++++++--------------
 5 files changed, 94 insertions(+), 70 deletions(-)

diff --git a/scripts/Dockerfile.alpine-base b/scripts/Dockerfile.alpine-base
index e7d5b916d..02608f1d4 100644
--- a/scripts/Dockerfile.alpine-base
+++ b/scripts/Dockerfile.alpine-base
@@ -51,5 +51,4 @@ RUN apk add --update --no-cache build-base wget git gcc cmake make yaml curl lib
 COPY --from=builder /usr/local/include/iot /usr/local/include/iot
 COPY --from=builder /usr/local/include/edgex /usr/local/include/edgex
 COPY --from=builder /usr/local/lib /usr/local/lib
-COPY --from=builder /usr/local/lib64 /usr/local/lib64
 COPY --from=builder /usr/local/share/device-sdk-c /usr/local/share/device-sdk-c
diff --git a/src/c/autoevent.c b/src/c/autoevent.c
index 7386aeb0f..f74b1a034 100644
--- a/src/c/autoevent.c
+++ b/src/c/autoevent.c
@@ -124,7 +124,10 @@ static void *ae_runner (void *p)
   {
     iot_log_error
       (ai->svc->logger, "Autoevent fired for unknown device %s", ai->device);
-    iot_schedule_remove (ai->svc->scheduler, ai->handle);
+    if (ai->handle)
+    {
+      iot_schedule_remove (ai->svc->scheduler, ai->handle);
+    }
   }
   edgex_autoimpl_release (ai);
   return NULL;
@@ -193,34 +196,22 @@ void edgex_device_autoevent_start (devsdk_service_t *svc, edgex_device *dev)
   }
 }
 
-static void *stopper (void *p)
+static void stopper (edgex_autoimpl *ai)
 {
-  edgex_autoimpl *ai = (edgex_autoimpl *)p;
+  void *handle = ai->handle;
+  ai->handle = NULL;
   if (ai->svc->userfns.ae_stopper)
   {
-    ai->svc->userfns.ae_stopper (ai->svc->userdata, ai->handle);
+    ai->svc->userfns.ae_stopper (ai->svc->userdata, handle);
   }
   else
   {
-    iot_schedule_delete (ai->svc->scheduler, ai->handle);
+    iot_schedule_delete (ai->svc->scheduler, handle);
   }
   edgex_autoimpl_release (ai);
-  return NULL;
 }
 
 void edgex_device_autoevent_stop (edgex_device *dev)
-{
-  for (edgex_device_autoevents *ae = dev->autos; ae; ae = ae->next)
-  {
-    if (ae->impl)
-    {
-      edgex_autoimpl *ai = ae->impl;
-      iot_threadpool_add_work (ai->svc->thpool, stopper, ai, -1);
-    }
-  }
-}
-
-void edgex_device_autoevent_stop_now (edgex_device *dev)
 {
   for (edgex_device_autoevents *ae = dev->autos; ae; ae = ae->next)
   {
diff --git a/src/c/autoevent.h b/src/c/autoevent.h
index 79e6fe2d3..b6c3be08e 100644
--- a/src/c/autoevent.h
+++ b/src/c/autoevent.h
@@ -14,6 +14,5 @@
 void edgex_device_autoevent_start (devsdk_service_t *svc, edgex_device *dev);
 
 void edgex_device_autoevent_stop (edgex_device *dev);
-void edgex_device_autoevent_stop_now (edgex_device *dev);
 
 #endif
diff --git a/src/c/devmap.c b/src/c/devmap.c
index 635f2f1db..a670607c2 100644
--- a/src/c/devmap.c
+++ b/src/c/devmap.c
@@ -199,7 +199,6 @@ static void remove_locked (edgex_devmap_t *map, edgex_device *olddev)
     edgex_map_remove (&map->profiles, olddev->profile->name);
     olddev->ownprofile = true;
   }
-  edgex_device_release (map->svc, olddev);
 }
 
 /* Update a device, but fail if there could be effects on autoevents or
@@ -240,25 +239,33 @@ static bool update_in_place (edgex_device *dest, const edgex_device *src, edgex_
 
 edgex_devmap_outcome_t edgex_devmap_replace_device (edgex_devmap_t *map, const edgex_device *dev)
 {
-  edgex_device **olddev;
+  edgex_device **od;
+  edgex_device *olddev;
+  bool release = false;
   edgex_devmap_outcome_t result = UPDATED_SDK;
 
   pthread_rwlock_wrlock (&map->lock);
-  olddev = edgex_map_get (&map->devices, dev->name);
-  if (olddev == NULL)
+  od = edgex_map_get (&map->devices, dev->name);
+  if (od == NULL)
   {
     add_locked (map, dev);
     result = CREATED;
   }
   else
   {
-    if (!update_in_place (*olddev, dev, &result))
+    olddev = *od;
+    if (!update_in_place (olddev, dev, &result))
     {
-      remove_locked (map, *olddev);
+      remove_locked (map, olddev);
       add_locked (map, dev);
+      release = true;
     }
   }
   pthread_rwlock_unlock (&map->lock);
+  if (release)
+  {
+    edgex_device_release (map->svc, olddev);
+  }
   return result;
 }
 
@@ -289,16 +296,26 @@ bool edgex_devmap_device_exists (edgex_devmap_t *map, const char *name)
 
 bool edgex_devmap_removedevice_byname (edgex_devmap_t *map, const char *name)
 {
-  edgex_device **olddev;
+  edgex_device **od;
+  edgex_device *olddev = NULL;
 
   pthread_rwlock_wrlock (&map->lock);
-  olddev = edgex_map_get (&map->devices, name);
-  if (olddev)
+  od = edgex_map_get (&map->devices, name);
+  if (od)
   {
-    remove_locked (map, *olddev);
+    olddev = *od;
+    remove_locked (map, olddev);
   }
   pthread_rwlock_unlock (&map->lock);
-  return (olddev != NULL);
+  if (olddev)
+  {
+    edgex_device_release (map->svc, olddev);
+    return true;
+  }
+  else
+  {
+    return false;
+  }
 }
 
 void edgex_devmap_add_profile (edgex_devmap_t *map, edgex_deviceprofile *dp)
@@ -322,7 +339,7 @@ void edgex_devmap_update_profile (devsdk_service_t *svc, edgex_deviceprofile *dp
       edgex_device **dev = edgex_map_get (&svc->devices->devices, key);
       if ((*dev)->profile == old)
       {
-        edgex_device_autoevent_stop_now (*dev);
+        edgex_device_autoevent_stop (*dev);
         (*dev)->profile = dp;
         edgex_device_autoevent_start (svc, *dev);
       }
diff --git a/src/c/edgex-rest.c b/src/c/edgex-rest.c
index ba27accca..349f001b5 100644
--- a/src/c/edgex-rest.c
+++ b/src/c/edgex-rest.c
@@ -432,16 +432,20 @@ static edgex_deviceresource *deviceresource_read
 static edgex_deviceresource *edgex_deviceresource_dup (const edgex_deviceresource *e)
 {
   edgex_deviceresource *result = NULL;
-  if (e)
+  edgex_deviceresource **current = &result;
+  while (e)
   {
-    result = malloc (sizeof (edgex_deviceresource));
-    result->name = strdup (e->name);
-    result->description = strdup (e->description);
-    result->tag = strdup (e->tag);
-    result->properties = propertyvalue_dup (e->properties);
-    result->attributes = iot_data_copy (e->attributes);
-    result->parsed_attrs = NULL;
-    result->next = edgex_deviceresource_dup (e->next);
+    edgex_deviceresource *elem = malloc (sizeof (edgex_deviceresource));
+    elem->name = strdup (e->name);
+    elem->description = strdup (e->description);
+    elem->tag = strdup (e->tag);
+    elem->properties = propertyvalue_dup (e->properties);
+    elem->attributes = iot_data_copy (e->attributes);
+    elem->parsed_attrs = NULL;
+    elem->next = NULL;
+    *current = elem;
+    current = &(elem->next);
+    e = e->next;
   }
   return result;
 }
@@ -478,17 +482,20 @@ static edgex_resourceoperation *resourceoperation_read (const JSON_Object *obj)
   return result;
 }
 
-static edgex_resourceoperation *resourceoperation_dup
-  (const edgex_resourceoperation *ro)
+static edgex_resourceoperation *resourceoperation_dup (const edgex_resourceoperation *ro)
 {
   edgex_resourceoperation *result = NULL;
-  if (ro)
+  edgex_resourceoperation **current = &result;
+  while (ro)
   {
-    result = malloc (sizeof (edgex_resourceoperation));
-    result->deviceResource = strdup (ro->deviceResource);
-    result->defaultValue = strdup (ro->defaultValue);
-    result->mappings = devsdk_nvpairs_dup (ro->mappings);
-    result->next = resourceoperation_dup (ro->next);
+    edgex_resourceoperation *elem = malloc (sizeof (edgex_resourceoperation));
+    elem->deviceResource = strdup (ro->deviceResource);
+    elem->defaultValue = strdup (ro->defaultValue);
+    elem->mappings = devsdk_nvpairs_dup (ro->mappings);
+    elem->next = NULL;
+    *current = elem;
+    current = &(elem->next);
+    ro = ro->next;
   }
   return result;
 }
@@ -533,14 +540,18 @@ static edgex_devicecommand *devicecommand_read (const JSON_Object *obj)
 static edgex_devicecommand *devicecommand_dup (const edgex_devicecommand *pr)
 {
   edgex_devicecommand *result = NULL;
-  if (pr)
-  {
-    result = malloc (sizeof (edgex_devicecommand));
-    result->name = strdup (pr->name);
-    result->readable = pr->readable;
-    result->writable = pr->writable;
-    result->resourceOperations = resourceoperation_dup (pr->resourceOperations);
-    result->next = devicecommand_dup (pr->next);
+  edgex_devicecommand **current = &result;
+  while (pr)
+  {
+    edgex_devicecommand *elem = malloc (sizeof (edgex_devicecommand));
+    elem->name = strdup (pr->name);
+    elem->readable = pr->readable;
+    elem->writable = pr->writable;
+    elem->resourceOperations = resourceoperation_dup (pr->resourceOperations);
+    elem->next = NULL;
+    *current = elem;
+    current = &(elem->next);
+    pr = pr->next;
   }
   return result;
 }
@@ -656,9 +667,9 @@ edgex_deviceprofile *edgex_deviceprofile_read (iot_logger_t *lc, const char *jso
 
 static void cmdinfo_free (edgex_cmdinfo *inf)
 {
-  if (inf)
+  while (inf)
   {
-    cmdinfo_free (inf->next);
+    edgex_cmdinfo *next = inf->next;
     for (unsigned i = 0; i < inf->nreqs; i++)
     {
       free (inf->reqs[i].resource);
@@ -668,6 +679,7 @@ static void cmdinfo_free (edgex_cmdinfo *inf)
     free (inf->maps);
     free (inf->dfls);
     free (inf);
+    inf = next;
   }
 }
 
@@ -703,26 +715,31 @@ static edgex_device_autoevents *autoevents_dup
   (const edgex_device_autoevents *e)
 {
   edgex_device_autoevents *result = NULL;
-  if (e)
+  edgex_device_autoevents **current = &result;
+  while (e)
   {
-    result = malloc (sizeof (edgex_device_autoevents));
-    result->resource = strdup (e->resource);
-    result->interval = strdup (e->interval);
-    result->onChange = e->onChange;
-    result->impl = NULL;
-    result->next = autoevents_dup (e->next);
+    edgex_device_autoevents *elem = malloc (sizeof (edgex_device_autoevents));
+    elem->resource = strdup (e->resource);
+    elem->interval = strdup (e->interval);
+    elem->onChange = e->onChange;
+    elem->impl = NULL;
+    elem->next = NULL;
+    *current = elem;
+    current = &(elem->next);
+    e = e->next;
   }
   return result;
 }
 
 void edgex_device_autoevents_free (edgex_device_autoevents *e)
 {
-  if (e)
+  while (e)
   {
+    edgex_device_autoevents *next = e->next;
     free (e->resource);
     free (e->interval);
-    edgex_device_autoevents_free (e->next);
     free (e);
+    e = next;
   }
 }
 
@@ -770,12 +787,13 @@ devsdk_protocols *devsdk_protocols_dup (const devsdk_protocols *e)
 
 void devsdk_protocols_free (devsdk_protocols *e)
 {
-  if (e)
+  while (e)
   {
+    devsdk_protocols *next = e->next;
     free (e->name);
     iot_data_free (e->properties);
-    devsdk_protocols_free (e->next);
     free (e);
+    e = next;
   }
 }