From 8ac6ba83be4514b94132748428cef0bbef7726d7 Mon Sep 17 00:00:00 2001
From: zhongtao <zhongtao17@huawei.com>
Date: Sun, 29 Jan 2023 15:48:07 +0800
Subject: [PATCH 07/19] Delete meaningless thread creation and ensure the
 task_console_accept thread ends before destroying the io_thread

Signed-off-by: zhongtao <zhongtao17@huawei.com>
---
 src/cmd/isulad-shim/main.c    |  5 ++--
 src/cmd/isulad-shim/process.c | 52 ++++++++++++++++++++++-------------
 src/cmd/isulad-shim/process.h |  4 +--
 3 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/src/cmd/isulad-shim/main.c b/src/cmd/isulad-shim/main.c
index 3ab22d86..eedd8fda 100644
--- a/src/cmd/isulad-shim/main.c
+++ b/src/cmd/isulad-shim/main.c
@@ -98,6 +98,7 @@ int main(int argc, char **argv)
     int ret = SHIM_ERR;
     int efd = -1;
     process_t *p = NULL;
+    pthread_t tid_accept;
 
     g_log_fd = open_no_inherit(SHIM_LOG_NAME, O_CREAT | O_WRONLY | O_APPEND | O_SYNC, 0640);
     if (g_log_fd < 0) {
@@ -151,7 +152,7 @@ int main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
-    ret = open_io(p);
+    ret = open_io(p, &tid_accept);
     if (ret != SHIM_OK) {
         exit(EXIT_FAILURE);
     }
@@ -166,5 +167,5 @@ int main(int argc, char **argv)
 
     released_timeout_exit();
 
-    return process_signal_handle_routine(p);
+    return process_signal_handle_routine(p, tid_accept);
 }
diff --git a/src/cmd/isulad-shim/process.c b/src/cmd/isulad-shim/process.c
index 92dd2701..66c604f3 100644
--- a/src/cmd/isulad-shim/process.c
+++ b/src/cmd/isulad-shim/process.c
@@ -423,6 +423,14 @@ static int start_io_copy_threads(process_t *p)
 
     /* 4 threads for stdin, stdout, stderr and exec resize */
     for (i = 0; i < 4; i++) {
+        /*
+        * if the terminal is used, we do not need to active the io copy of stderr pipe,
+        * for stderr and stdout are mixed together
+        */
+        if (i == STDID_ERR && p->state->terminal) {
+            continue;
+        }
+
         ret = create_io_copy_thread(p, i);
         if (ret != SHIM_OK) {
             return SHIM_ERR;
@@ -512,11 +520,6 @@ static void *task_console_accept(void *data)
     int ret = SHIM_ERR;
     console_accept_t *ac = (console_accept_t *)data;
 
-    if ((pthread_detach(pthread_self())) != 0) {
-        write_message(g_log_fd, ERR_MSG, "detach thread failed");
-        return NULL;
-    }
-
     conn_fd = accept(ac->listen_fd, NULL, NULL);
     if (conn_fd < 0) {
         write_message(g_log_fd, ERR_MSG, "accept from fd %d failed:%d", ac->listen_fd, SHIM_SYS_ERR(errno));
@@ -549,12 +552,6 @@ static void *task_console_accept(void *data)
         goto out;
     }
 
-    /*
-     * if the terminal is used, we do not need to active the io copy of stderr pipe,
-     * for stderr and stdout are mixed together
-     */
-    destroy_io_thread(ac->p, STDID_ERR);
-
 out:
     /* release listen socket at the first time */
     close_fd(&ac->listen_fd);
@@ -634,7 +631,7 @@ static int new_temp_console_path(process_t *p)
     return SHIM_OK;
 }
 
-static int console_init(process_t *p)
+static int console_init(process_t *p, pthread_t *tid_accept)
 {
     int ret = SHIM_ERR;
     int fd = -1;
@@ -670,8 +667,7 @@ static int console_init(process_t *p)
     ac->p = p;
     ac->listen_fd = fd;
 
-    pthread_t tid_accept;
-    ret = pthread_create(&tid_accept, NULL, task_console_accept, ac);
+    ret = pthread_create(tid_accept, NULL, task_console_accept, ac);
     if (ret != SHIM_OK) {
         goto failure;
     }
@@ -762,7 +758,7 @@ failure:
     return NULL;
 }
 
-static int open_terminal_io(process_t *p)
+static int open_terminal_io(process_t *p, pthread_t *tid_accept)
 {
     int ret = SHIM_ERR;
 
@@ -773,7 +769,7 @@ static int open_terminal_io(process_t *p)
     }
 
     /* begin listen and accept fd from p->console_sock_path */
-    return console_init(p);
+    return console_init(p, tid_accept);
 }
 
 static int open_generic_io(process_t *p)
@@ -916,7 +912,7 @@ failure:
     return NULL;
 }
 
-int open_io(process_t *p)
+int open_io(process_t *p, pthread_t *tid_accept)
 {
     int ret = SHIM_ERR;
 
@@ -926,7 +922,7 @@ int open_io(process_t *p)
     }
 
     if (p->state->terminal) {
-        return open_terminal_io(p);
+        return open_terminal_io(p, tid_accept);
     }
 
     return open_generic_io(p);
@@ -1216,12 +1212,13 @@ static int try_wait_all_child(void) {
     return 1;
 }
 
-int process_signal_handle_routine(process_t *p)
+int process_signal_handle_routine(process_t *p, const pthread_t tid_accept)
 {
     int ret = SHIM_ERR;
     bool exit_shim = false;
     int nret = 0;
     int i;
+    struct timespec ts;
 
     for (;;) {
         int status;
@@ -1257,6 +1254,23 @@ int process_signal_handle_routine(process_t *p)
             if (p->exit_fd > 0) {
                 (void)write_nointr(p->exit_fd, &status, sizeof(int));
             }
+            // wait for task_console_accept thread termination. In order to make sure that
+            // the io_copy connection is established and io_thread is not used by multiple threads.
+            if (p->state->terminal) {
+                if (clock_gettime(CLOCK_REALTIME, &ts) == -1) {
+                    write_message(g_log_fd, ERR_MSG, "Failed to get realtime");
+                    nret = pthread_join(tid_accept, NULL);
+                } else {
+                    // Set the maximum waiting time to 60s to prevent stuck.
+                    ts.tv_sec += 60;
+                    nret = pthread_timedjoin_np(tid_accept, NULL, &ts);
+                }
+
+                if (nret != 0) {
+                    write_message(g_log_fd, ERR_MSG, "Failed to join task_console_accept thread");
+                }
+            }
+
             for (i = 0; i < 3; i++) {
                 destroy_io_thread(p, i);
             }
diff --git a/src/cmd/isulad-shim/process.h b/src/cmd/isulad-shim/process.h
index 11d1bf64..66820f68 100644
--- a/src/cmd/isulad-shim/process.h
+++ b/src/cmd/isulad-shim/process.h
@@ -94,10 +94,10 @@ typedef struct {
 
 process_t* new_process(char *id, char *bundle, char *runtime);
 
-int open_io(process_t *p);
+int open_io(process_t *p, pthread_t *tid_accept);
 int process_io_init(process_t *p);
 int create_process(process_t *p);
-int process_signal_handle_routine(process_t *p);
+int process_signal_handle_routine(process_t *p, const pthread_t tid_accept);
 
 #ifdef __cplusplus
 }
-- 
2.25.1

