8202884: SA: Attach/detach might fail on Linux if debugee application...

8202884: SA: Attach/detach might fail on Linux if debugee application create/destroy threads during attaching
Summary: While doing a ptrace attach, do not attach to threads which are in the process of exiting or are zombies -- skip these threads.
Reviewed-by: jcbeyler, ysuenaga

agent/src/os/linux/libproc_impl.c

 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -256,6 +256,26 @@ thread_info* add_thread_info(struct ps_prochandle* ph, pthread_t pthread_id, lwp
    return newthr;
 }
 
+void delete_thread_info(struct ps_prochandle* ph, thread_info* thr_to_be_removed) {
+    thread_info* current_thr = ph->threads;
+
+    if (thr_to_be_removed == ph->threads) {
+      ph->threads = ph->threads->next;
+    } else {
+      thread_info* previous_thr;
+      while (current_thr && current_thr != thr_to_be_removed) {
+        previous_thr = current_thr;
+        current_thr = current_thr->next;
+      }
+      if (current_thr == NULL) {
+        print_error("Could not find the thread to be removed");
+        return;
+      }
+      previous_thr->next = current_thr->next;
+    }
+    ph->num_threads--;
+    free(current_thr);
+}
 
 // struct used for client data from thread_db callback
 struct thread_db_client_data {
@@ -278,6 +298,11 @@ static int thread_db_callback(const td_thrhandle_t *th_p, void *data) {
 
   print_debug("thread_db : pthread %d (lwp %d)\n", ti.ti_tid, ti.ti_lid);
 
+  if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE) {
+    print_debug("Skipping pthread %d (lwp %d)\n", ti.ti_tid, ti.ti_lid);
+    return TD_OK;
+  }
+
   if (ptr->callback(ptr->ph, ti.ti_tid, ti.ti_lid) != true)
     return TD_ERR;
 

agent/src/os/linux/libproc_impl.h

 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -113,6 +113,9 @@ typedef bool (*thread_info_callback)(struct ps_prochandle* ph, pthread_t pid, lw
 // reads thread info using libthread_db and calls above callback for each thread
 bool read_thread_info(struct ps_prochandle* ph, thread_info_callback cb);
 
+// deletes a thread from the thread list
+void delete_thread_info(struct ps_prochandle* ph, thread_info* thr);
+
 // adds a new shared object to lib list, returns NULL on failure
 lib_info* add_lib_info(struct ps_prochandle* ph, const char* libname, uintptr_t base);
 

agent/src/os/linux/ps_proc.c

@@ -28,6 +28,7 @@
 #include <signal.h>
 #include <errno.h>
 #include <elf.h>
+#include <ctype.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <sys/ptrace.h>
@@ -45,6 +46,12 @@
 // This file has the libproc implementation specific to live process
 // For core files, refer to ps_core.c
 
+typedef enum {
+  ATTACH_SUCCESS,
+  ATTACH_FAIL,
+  ATTACH_THREAD_DEAD
+} attach_state_t;
+
 static inline uintptr_t align(uintptr_t ptr, size_t size) {
   return (ptr & ~(size - 1));
 }
@@ -167,9 +174,10 @@ static bool ptrace_continue(pid_t pid, int signal) {
 
 // waits until the ATTACH has stopped the process
 // by signal SIGSTOP
-static bool ptrace_waitpid(pid_t pid) {
+static attach_state_t ptrace_waitpid(pid_t pid) {
   int ret;
   int status;
+  errno = 0;
   while (true) {
     // Wait for debuggee to stop.
     ret = waitpid(pid, &status, 0);
@@ -184,15 +192,15 @@ static bool ptrace_waitpid(pid_t pid) {
         // will go to sleep.
         if (WSTOPSIG(status) == SIGSTOP) {
           // Debuggee stopped by SIGSTOP.
-          return true;
+          return ATTACH_SUCCESS;
         }
         if (!ptrace_continue(pid, WSTOPSIG(status))) {
           print_error("Failed to correctly attach to VM. VM might HANG! [PTRACE_CONT failed, stopped by %d]\n", WSTOPSIG(status));
-          return false;
+          return ATTACH_FAIL;
         }
       } else {
-        print_debug("waitpid(): Child process exited/terminated (status = 0x%x)\n", status);
-        return false;
+        print_debug("waitpid(): Child process %d exited/terminated (status = 0x%x)\n", pid, status);
+        return ATTACH_THREAD_DEAD;
       }
     } else {
       switch (errno) {
@@ -201,29 +209,89 @@ static bool ptrace_waitpid(pid_t pid) {
           break;
         case ECHILD:
           print_debug("waitpid() failed. Child process pid (%d) does not exist \n", pid);
-          break;
+          return ATTACH_THREAD_DEAD;
         case EINVAL:
-          print_debug("waitpid() failed. Invalid options argument.\n");
-          break;
+          print_error("waitpid() failed. Invalid options argument.\n");
+          return ATTACH_FAIL;
         default:
-          print_debug("waitpid() failed. Unexpected error %d\n",errno);
-          break;
+          print_error("waitpid() failed. Unexpected error %d\n",errno);
+          return ATTACH_FAIL;
       }
-      return false;
+    } // else
+  } // while
+}
+
+// checks the state of the thread/process specified by "pid", by reading
+// in the 'State:' value from the /proc/<pid>/status file. From the proc
+// man page, "Current state of the process. One of "R (running)",
+// "S (sleeping)", "D (disk sleep)", "T (stopped)", "T (tracing stop)",
+// "Z (zombie)", or "X (dead)"." Assumes that the thread is dead if we
+// don't find the status file or if the status is 'X' or 'Z'.
+static bool process_doesnt_exist(pid_t pid) {
+  char fname[32];
+  char buf[30];
+  FILE *fp = NULL;
+  const char state_string[] = "State:";
+
+  sprintf(fname, "/proc/%d/status", pid);
+  fp = fopen(fname, "r");
+  if (fp == NULL) {
+    print_debug("can't open /proc/%d/status file\n", pid);
+    // Assume the thread does not exist anymore.
+    return true;
+  }
+  bool found_state = false;
+  size_t state_len = strlen(state_string);
+  while (fgets(buf, sizeof(buf), fp) != NULL) {
+    char *state = NULL;
+    if (strncmp (buf, state_string, state_len) == 0) {
+      found_state = true;
+      state = buf + state_len;
+      // Skip the spaces
+      while (isspace(*state)) {
+        state++;
+      }
+      // A state value of 'X' indicates that the thread is dead. 'Z'
+      // indicates that the thread is a zombie.
+      if (*state == 'X' || *state == 'Z') {
+        fclose (fp);
+        return true;
+      }
+      break;
     }
   }
+  // If the state value is not 'X' or 'Z', the thread exists.
+  if (!found_state) {
+    // We haven't found the line beginning with 'State:'.
+    // Assuming the thread exists.
+    print_error("Could not find the 'State:' string in the /proc/%d/status file\n", pid);
+  }
+  fclose (fp);
+  return false;
 }
 
 // attach to a process/thread specified by "pid"
-static bool ptrace_attach(pid_t pid, char* err_buf, size_t err_buf_len) {
+static attach_state_t ptrace_attach(pid_t pid, char* err_buf, size_t err_buf_len) {
+  errno = 0;
   if (ptrace(PTRACE_ATTACH, pid, NULL, NULL) < 0) {
+    if (errno == EPERM || errno == ESRCH) {
+      // Check if the process/thread is exiting or is a zombie
+      if (process_doesnt_exist(pid)) {
+        print_debug("Thread with pid %d does not exist\n", pid);
+        return ATTACH_THREAD_DEAD;
+      }
+    }
     char buf[200];
     char* msg = strerror_r(errno, buf, sizeof(buf));
     snprintf(err_buf, err_buf_len, "ptrace(PTRACE_ATTACH, ..) failed for %d: %s", pid, msg);
-    print_debug("%s\n", err_buf);
-    return false;
+    print_error("%s\n", err_buf);
+    return ATTACH_FAIL;
   } else {
-    return ptrace_waitpid(pid);
+    attach_state_t wait_ret = ptrace_waitpid(pid);
+    if (wait_ret == ATTACH_THREAD_DEAD) {
+      print_debug("Thread with pid %d does not exist\n", pid);
+    }
+    return wait_ret;
   }
 }
 
@@ -345,16 +413,20 @@ static ps_prochandle_ops process_ops = {
 struct ps_prochandle* Pgrab(pid_t pid, char* err_buf, size_t err_buf_len) {
   struct ps_prochandle* ph = NULL;
   thread_info* thr = NULL;
+  attach_state_t attach_status = ATTACH_SUCCESS;
 
   if ( (ph = (struct ps_prochandle*) calloc(1, sizeof(struct ps_prochandle))) == NULL) {
-     snprintf(err_buf, err_buf_len, "can't allocate memory for ps_prochandle");
-     print_debug("%s\n", err_buf);
-     return NULL;
+    snprintf(err_buf, err_buf_len, "can't allocate memory for ps_prochandle");
+    print_debug("%s\n", err_buf);
+    return NULL;
   }
 
-  if (ptrace_attach(pid, err_buf, err_buf_len) != true) {
-     free(ph);
-     return NULL;
+  if ((attach_status = ptrace_attach(pid, err_buf, err_buf_len)) != ATTACH_SUCCESS) {
+    if (attach_status == ATTACH_THREAD_DEAD) {
+       print_error("The process with pid %d does not exist.\n", pid);
+    }
+    free(ph);
+    return NULL;
   }
 
   // initialize ps_prochandle
@@ -373,14 +445,23 @@ struct ps_prochandle* Pgrab(pid_t pid, char* err_buf, size_t err_buf_len) {
 
   // attach to the threads
   thr = ph->threads;
+
   while (thr) {
-     // don't attach to the main thread again
-    if (ph->pid != thr->lwp_id && ptrace_attach(thr->lwp_id, err_buf, err_buf_len) != true) {
-        // even if one attach fails, we get return NULL
-        Prelease(ph);
-        return NULL;
-     }
-     thr = thr->next;
+    thread_info* current_thr = thr;
+    thr = thr->next;
+    // don't attach to the main thread again
+    if (ph->pid != current_thr->lwp_id) {
+      if ((attach_status = ptrace_attach(current_thr->lwp_id, err_buf, err_buf_len)) != ATTACH_SUCCESS) {
+        if (attach_status == ATTACH_THREAD_DEAD) {
+          // Remove this thread from the threads list
+          delete_thread_info(ph, current_thr);
+        }
+        else {
+          Prelease(ph);
+          return NULL;
+        } // ATTACH_THREAD_DEAD
+      } // !ATTACH_SUCCESS
+    }
   }
   return ph;
 }