From db230b3dcb7bd0c197d430a2dfb742dccab9b971 Mon Sep 17 00:00:00 2001
From: huanchen <huanchen@qti.qualcomm.com>
Date: Mon, 17 Jul 2023 15:49:00 +0800
Subject: [PATCH] ipc: Unmap shared memory before destroy

---
 src/xrt/ipc/client/ipc_client_instance.c |  3 ++
 src/xrt/ipc/server/ipc_server_process.c  |  2 ++
 src/xrt/ipc/shared/ipc_shmem.c           | 40 +++++++++++++++++++++---
 src/xrt/ipc/shared/ipc_shmem.h           | 20 ++++++++++--
 4 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/src/xrt/ipc/client/ipc_client_instance.c b/src/xrt/ipc/client/ipc_client_instance.c
index a5f26947a..75146361c 100644
--- a/src/xrt/ipc/client/ipc_client_instance.c
+++ b/src/xrt/ipc/client/ipc_client_instance.c
@@ -25,6 +25,7 @@
 #include "util/u_system_helpers.h"
 
 #include "shared/ipc_protocol.h"
+#include "shared/ipc_shmem.h"
 #include "client/ipc_client.h"
 #include "client/ipc_client_connection.h"
 
@@ -202,6 +203,8 @@ ipc_client_instance_destroy(struct xrt_instance *xinst)
 	}
 	ii->xtrack_count = 0;
 
+	ipc_shmem_destroy(&ii->ipc_c.ism_handle, (void **)&ii->ipc_c.ism, sizeof(struct ipc_shared_memory));
+
 	free(ii);
 }
 
diff --git a/src/xrt/ipc/server/ipc_server_process.c b/src/xrt/ipc/server/ipc_server_process.c
index a06c3f4a7..4a683cdb7 100644
--- a/src/xrt/ipc/server/ipc_server_process.c
+++ b/src/xrt/ipc/server/ipc_server_process.c
@@ -141,6 +141,8 @@ teardown_all(struct ipc_server *s)
 	u_process_destroy(s->process);
 
 	os_mutex_destroy(&s->global_state.lock);
+
+	ipc_shmem_destroy(&s->ism_handle, (void **)&s->ism, sizeof(struct ipc_shared_memory));
 }
 
 static int
diff --git a/src/xrt/ipc/shared/ipc_shmem.c b/src/xrt/ipc/shared/ipc_shmem.c
index 2dd1b60de..5778d6150 100644
--- a/src/xrt/ipc/shared/ipc_shmem.c
+++ b/src/xrt/ipc/shared/ipc_shmem.c
@@ -105,9 +105,13 @@ ipc_shmem_create(size_t size, xrt_shmem_handle_t *out_handle, void **out_map)
 #endif
 
 #if defined(XRT_OS_UNIX)
+
 void
-ipc_shmem_destroy(xrt_shmem_handle_t *handle_ptr)
+ipc_shmem_destroy(xrt_shmem_handle_t *handle_ptr, void **map_ptr, size_t size)
 {
+	// Checks for NULL.
+	ipc_shmem_unmap((void **)map_ptr, size);
+
 	if (handle_ptr == NULL) {
 		return;
 	}
@@ -132,10 +136,25 @@ ipc_shmem_map(xrt_shmem_handle_t handle, size_t size, void **out_map)
 	*out_map = ptr;
 	return XRT_SUCCESS;
 }
-#elif defined(XRT_OS_WINDOWS)
+
 void
-ipc_shmem_destroy(xrt_shmem_handle_t *handle_ptr)
+ipc_shmem_unmap(void **map_ptr, size_t size)
 {
+	if (map_ptr == NULL) {
+		return;
+	}
+	munmap(*map_ptr, size);
+	*map_ptr = NULL;
+}
+
+#elif defined(XRT_OS_WINDOWS)
+
+void
+ipc_shmem_destroy(xrt_shmem_handle_t *handle_ptr, void **map_ptr, size_t size)
+{
+	// Checks for NULL.
+	ipc_shmem_unmap((void **)map_ptr, size);
+
 	if (handle_ptr == NULL) {
 		return;
 	}
@@ -155,7 +174,20 @@ ipc_shmem_map(xrt_shmem_handle_t handle, size_t size, void **out_map)
 	return XRT_SUCCESS;
 }
 
-// BUGBUG: unmap?
+void
+ipc_shmem_unmap(void **map_ptr, size_t size)
+{
+	if (map_ptr == NULL) {
+		return;
+	}
+	void *map = *map_ptr;
+	if (map == NULL) {
+		return;
+	}
+	UnmapViewOfFile(map);
+	*map_ptr = NULL;
+}
+
 #else
 #error "OS not yet supported"
 #endif
diff --git a/src/xrt/ipc/shared/ipc_shmem.h b/src/xrt/ipc/shared/ipc_shmem.h
index c7596f461..92372dc2f 100644
--- a/src/xrt/ipc/shared/ipc_shmem.h
+++ b/src/xrt/ipc/shared/ipc_shmem.h
@@ -16,6 +16,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -51,6 +52,17 @@ ipc_shmem_create(size_t size, xrt_shmem_handle_t *out_handle, void **out_map);
 xrt_result_t
 ipc_shmem_map(xrt_shmem_handle_t handle, size_t size, void **out_map);
 
+/*!
+ * Unmap a shared memory region.
+ *
+ * @param[in] map_ptr pointer to region
+ * @param[in] size Size of region
+ *
+ * @public @memberof xrt_shmem_handle_t
+ */
+void
+ipc_shmem_unmap(void **map_ptr, size_t size);
+
 /*!
  * Destroy a handle to a shared memory region.
  *
@@ -58,12 +70,16 @@ ipc_shmem_map(xrt_shmem_handle_t handle, size_t size, void **out_map);
  * it (in this process or others) are still open.
  *
  * @param[in,out] handle_ptr Pointer to the handle to destroy - will be checked
- * for validity, destroyed, and cleared.
+ *                           for validity, destroyed, and cleared.
+ * @param[in,out] map_ptr Pointer to the mapped memory to unmap - will be
+ *                        checked for validity, destroyed, and cleared. It's
+ *                        necessary unmap the region to destroy the shmem.
+ * @param[in] size Size of the mapped region.
  *
  * @public @memberof xrt_shmem_handle_t
  */
 void
-ipc_shmem_destroy(xrt_shmem_handle_t *handle_ptr);
+ipc_shmem_destroy(xrt_shmem_handle_t *handle_ptr, void **map_ptr, size_t size);
 
 #ifdef __cplusplus
 }