[LibOS/ipc] async helper: keep refcount to helper thread on exit

Using put_thread(self) to free resources by itself is problematic because the exiting thread is still using them. Actually, put_thread(self) logic in ipc/async helper may cause SEGV depending on who is the last to put_thread(). If the thread is the last one to free thread area, lock/unlock after freeing thread area causes SEGV due to debug message.

The solution is to keep reference count of those two helper threads by a (usually master) thread so that the (master) thread will be the last thread to release the reference count. This way self destruction can be avoided.

To completely fix this issue, helper threads need to be waited to exit.
The issue is tracked by https://github.com/oscarlab/graphene/issues/440.

Signed-off-by: Isaku Yamahata <isaku.yamahata@gmail.com>

LibOS/shim/include/shim_ipc.h

@@ -604,7 +604,7 @@ void ipc_parent_exit  (struct shim_ipc_port * port, IDTYPE vmid,
 void ipc_child_exit   (struct shim_ipc_port * port, IDTYPE vmid,
                        unsigned int exitcode);
 
-int exit_with_ipc_helper (bool handover);
+int exit_with_ipc_helper (bool handover, struct shim_thread ** ret);
 
 #define IPC_FORCE_RECONNECT     ((void *) -1)
 

LibOS/shim/include/shim_utils.h

@@ -243,7 +243,7 @@ int64_t install_async_event (PAL_HANDLE object, unsigned long time,
                               void (*callback) (IDTYPE caller, void * arg),
                               void * arg);
 int create_async_helper (void);
-int terminate_async_helper (void);
+struct shim_thread * terminate_async_helper (void);
 
 extern struct config_store * root_config;
 

LibOS/shim/src/ipc/shim_ipc_helper.c

@@ -1079,8 +1079,19 @@ static int create_ipc_helper (void)
     return 0;
 }
 
-int exit_with_ipc_helper (bool handover)
+/*
+ * on success, the reference to the helper thread is returned with
+ * reference count incremented.
+ * The caller is responsible to wait for the IPC helper thread to exit
+ * and release the final reference to free related resources.
+ * It's problematic for the thread itself to release its resources which it's
+ * using. For example stack.
+ * So defer releasing it after its exit and make the releasing the caller
+ * responsibility.
+ */
+int exit_with_ipc_helper (bool handover, struct shim_thread ** ret)
 {
+    *ret = NULL;
     if (IN_HELPER() || ipc_helper_state != HELPER_ALIVE)
         return 0;
 
@@ -1104,6 +1115,10 @@ int exit_with_ipc_helper (bool handover)
     }
 
     ipc_helper_state = new_state;
+    if (ipc_helper_thread != NULL) {
+        get_thread(ipc_helper_thread);
+        *ret = ipc_helper_thread;
+    }
     unlock(ipc_helper_lock);
 
     set_event(&ipc_helper_event, 1);

LibOS/shim/src/shim_async.c

@@ -318,14 +318,27 @@ int create_async_helper (void)
     return 0;
 }
 
-int terminate_async_helper (void)
+/*
+ * On success, the reference to the thread of async helper is returned with
+ * reference count incremented.
+ * It's caller the responsibility to wait for its exit and release the
+ * final reference to free related resources.
+ * It's problematic for the thread itself to release its resources which it's
+ * using. For example stack.
+ * So defer releasing it after its exit and make the releasing the caller
+ * responsibility.
+ */
+struct shim_thread * terminate_async_helper (void)
 {
     if (async_helper_state != HELPER_ALIVE)
-        return 0;
+        return NULL;
 
     lock(async_helper_lock);
+    struct shim_thread * ret = async_helper_thread;
+    if (ret)
+        get_thread(ret);
     async_helper_state = HELPER_NOTALIVE;
     unlock(async_helper_lock);
     set_event(&async_helper_event, 1);
-    return 0;
+    return ret;
 }

LibOS/shim/src/sys/shim_exit.c

@@ -142,9 +142,23 @@ int try_process_exit (int error_code, int term_signal)
     if (check_last_thread(cur_thread))
         return 0;
 
-    terminate_async_helper();
-
-    if (!exit_with_ipc_helper(true))
+    struct shim_thread * async_thread = terminate_async_helper();
+    if (async_thread)
+        /* TODO: wait for the thread to exit in host.
+         * This is tracked by the following issue.
+         * https://github.com/oscarlab/graphene/issues/440
+         */
+        put_thread(async_thread); /* free resources of the thread */
+
+    struct shim_thread * ipc_thread;
+    int ret = exit_with_ipc_helper(true, &ipc_thread);
+    if (ipc_thread)
+        /* TODO: wait for the thread to exit in host.
+         * This is tracked by the following issue.
+         * https://github.com/oscarlab/graphene/issues/440
+         */
+        put_thread(ipc_thread); /* free resources of the thread */
+    if (!ret)
         shim_clean();
     else
         DkThreadExit();