Merge pull request #1210 from atsign-foundation/xc/srv-zombie-process

fix: check for child process exits periodically

packages/c/sshnpd/include/sshnpd/handle_ssh_request.h

@@ -1,12 +1,14 @@
 #ifndef HANDLE_SSH_REQUEST_H
 #define HANDLE_SSH_REQUEST_H
 #include "sshnpd/params.h"
+#include "sshnpd/sshnpd.h"
 #include <atclient/monitor.h>
 #include <pthread.h>
 
 void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshnpd_params *params,
                         bool *is_child_process, atclient_monitor_message *message, char *home_dir, FILE *authkeys_file,
-                        char *authkeys_filename, atchops_rsakey_privatekey signing_key);
+                        char *authkeys_filename, atchops_rsakey_privatekey signing_key,
+                        struct sshnpd_process_queue *pids);
 
 int verify_envelope_signature(atchops_rsakey_publickey publickey, const unsigned char *payload,
                               unsigned char *signature, const char *hashing_algo, const char *signing_algo);

packages/c/sshnpd/include/sshnpd/sshnpd.h

@@ -1,8 +1,8 @@
 #ifndef SSHNPD_H
 #define SSHNPD_H
+#include <unistd.h>
 
 #define SSHNPD_VERSION "0.1.0"
-
 /* Windows Definitions */
 #ifdef _WIN32
 #define HOMEVAR "USERPROFILE"
@@ -32,6 +32,11 @@ enum notification_key {
   NK_NPT_REQUEST,
 };
 
+struct sshnpd_process_queue {
+  size_t len;
+  pid_t *processes;
+};
+
 #define NOTIFICATION_KEYS_LEN 5
 
 #endif

packages/c/sshnpd/src/handle_ssh_request.c

@@ -25,7 +25,8 @@
 
 void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshnpd_params *params,
                         bool *is_child_process, atclient_monitor_message *message, char *home_dir, FILE *authkeys_file,
-                        char *authkeys_filename, atchops_rsakey_privatekey signing_key) {
+                        char *authkeys_filename, atchops_rsakey_privatekey signing_key,
+                        struct sshnpd_process_queue *pids) {
   int res = 0;
   char *requesting_atsign = message->notification.from;
 
@@ -434,8 +435,8 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn
       free(session_iv_encrypted);
       free_session_base64 = true;
     } // rsa2048 - allocates (session_iv_base64, session_aes_key_base64)
-  }   // case 7
-  }   // switch
+  } // case 7
+  } // switch
 
   if (!is_valid) {
     atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR,
@@ -479,7 +480,22 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn
   } else if (pid > 0) {
 
     // parent process
-    waitpid(pid, &status, WNOHANG); // Don't wait for srv
+    waitpid(pid, &status, WNOHANG); // Don't wait for srv - we want it to be running in the bg
+    if (WIFEXITED(status)) {
+      goto cancel;
+    }
+
+    pids->processes = realloc(pids->processes, sizeof(pid_t) * (pids->len + 1));
+    if (pids->processes == NULL) {
+      // we leak ownership of srv here to the system... not much can be done if we run out of memory to track the
+      // process though
+      // TODO  - what should sshnpd do at this state?
+      // - shutdown everything?
+      // - try to kill the process we don't have enough memory to track
+      // - just move on and acccept that the process is srv and will self-shutdown eventually
+      goto cancel;
+    }
+    pids->processes[pids->len++] = pid;
 
     char *identifier = cJSON_GetStringValue(session_id);
     cJSON *final_res_payload = cJSON_CreateObject();
@@ -566,12 +582,12 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn
       atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Released the atclient lock\n");
     }
 
-  clean_res : { free(keyname); }
-  clean_final_res_value : {
+  clean_res: { free(keyname); }
+  clean_final_res_value: {
     atclient_atkey_free(&final_res_atkey);
     free(final_res_value);
   }
-  clean_json : {
+  clean_json: {
     cJSON_Delete(final_res_envelope);
     cJSON_free(signing_input2);
   }
@@ -614,4 +630,4 @@ int verify_envelope_signature(atchops_rsakey_publickey publickey, const unsigned
   atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "verify_envelope_signature (success)\n");
 
   return ret;
-}
+}

packages/c/sshnpd/src/main.c

@@ -29,6 +29,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/errno.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
 #define FILENAME_BUFFER_SIZE 500
@@ -84,6 +85,9 @@ static void sig_handler(int _) {
   }
 }
 
+// Queue for all srv processes
+static struct sshnpd_process_queue srv_pids = {0, 0, NULL};
+
 int main(int argc, char **argv) {
   int res = 0;
   int exit_res = 0;
@@ -202,10 +206,6 @@ int main(int argc, char **argv) {
     goto cancel_atclient;
   }
 
-  // pipe to communicate with the threads we will create in 9 & 10
-  int fds[2];
-  pipe(fds);
-
   cJSON *ping_response_json = cJSON_CreateObject();
 
   cJSON_AddItemToObject(ping_response_json, "devicename", cJSON_CreateString(params.device));
@@ -257,13 +257,13 @@ int main(int argc, char **argv) {
     goto clean_atkeys;
   }
 
-  for(int i = 0; i < params.manager_list_len; i++) {
+  for (int i = 0; i < params.manager_list_len; i++) {
     atclient_atkey_init(infokeys + i);
     atclient_atkey_init(usernamekeys + i);
   }
 
-  struct refresh_device_entry_params refresh_params = {&worker,       &atclient_lock, &params,
-                                                       ping_response, username,       &should_run, infokeys, usernamekeys};
+  struct refresh_device_entry_params refresh_params = {&worker,  &atclient_lock, &params,  ping_response,
+                                                       username, &should_run,    infokeys, usernamekeys};
   res = pthread_create(&refresh_tid, NULL, refresh_device_entry, (void *)&refresh_params);
   if (res != 0) {
     atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to start refresh device entry thread\n");
@@ -322,12 +322,22 @@ int main(int argc, char **argv) {
     goto close_authkeys;
   }
 
-  // 12. Main notification handler loop
+  // 12. Create child process queue
+  srv_pids.len = 0;
+  srv_pids.processes = malloc(sizeof(pid_t) * srv_pids.len);
+  if (srv_pids.processes == NULL) {
+    goto clean_device_info_keys;
+  }
+
+  // 13. Main notification handler loop
   atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_INFO, "Starting main loop\n");
   main_loop();
   atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_INFO, "Exited main loop\n");
 
-  for(int i = 0; i < params.manager_list_len; i++) {
+  free(srv_pids.processes);
+
+clean_device_info_keys:
+  for (int i = 0; i < params.manager_list_len; i++) {
     atclient_atkey_free(infokeys + i);
     atclient_atkey_free(usernamekeys + i);
   }
@@ -348,7 +358,6 @@ int main(int argc, char **argv) {
   } else {
     atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Joined device entry refresh thread\n");
   }
-
 cancel_atclient:
   if (free_ping_response) {
     free(ping_response);
@@ -390,6 +399,18 @@ void main_loop() {
   while (should_run) {
     atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Waiting for next monitor thread message\n");
     atclient_monitor_message_init(&message);
+
+    // Loop through background instances of sshnpd to see if they are ready to exit
+    int status;
+    for (int i = 0; i < srv_pids.len; i++) {
+      waitpid(srv_pids.processes[i], &status, WNOHANG); // Don't wait for srv - we want it to be running in the bg
+      if (WIFEXITED(status)) {
+        atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Exited srv process: %d\n", srv_pids.processes[i]);
+        srv_pids.processes = realloc(srv_pids.processes, sizeof(pid_t) * --srv_pids.len);
+      }
+    }
+
+    // Read the next monitor message
     res = atclient_monitor_read(&monitor_ctx, &worker, &message, &monitor_hooks);
 
     atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Received message of type: %d\n", message.type);
@@ -502,7 +523,7 @@ void main_loop() {
         case NK_SSH_REQUEST:
           atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Executing handle_ssh_request\n");
           handle_ssh_request(&worker, &atclient_lock, &params, &is_child_process, &message, home_dir, authkeys_file,
-                             authkeys_filename, signingkey);
+                             authkeys_filename, signingkey, &srv_pids);
           if (is_child_process) {
             atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Exiting child process\n");
             atclient_monitor_message_free(&message);

packages/c/sshnpd/src/run_srv_process.c

@@ -94,4 +94,4 @@ int run_srv_process(sshnpd_params *params, cJSON *host, cJSON *port, bool authen
   free(streaming_port);
 
   return res;
-}
+}