Merge remote-tracking branch 'tor-github/pr/1424' into maint-0.4.0

changes/bug31810

@@ -0,0 +1,4 @@
+  o Minor bugfixes (process management):
+    - Remove assertion in the Unix process backend. This assertion would trigger
+      when a new process is spawned where the executable is not found leading to
+      a stack trace from the child process. Fixes bug 31810; bugfix on 0.4.0.1-alpha.

src/lib/process/process_unix.c

@@ -253,22 +253,15 @@ process_unix_exec(process_t *process)
     process_environment_t *env = process_get_environment(process);
 
     /* Call the requested program. */
-    retval = execve(argv[0], argv, env->unixoid_environment_block);
+    execve(argv[0], argv, env->unixoid_environment_block);
 
     /* If we made it here it is because execve failed :-( */
-    if (-1 == retval)
-      fprintf(stderr, "Call to execve() failed: %s", strerror(errno));
-
     tor_free(argv);
     process_environment_free(env);
 
-    tor_assert_unreached();
-
  error:
-    /* LCOV_EXCL_START */
     fprintf(stderr, "Error from child process: %s", strerror(errno));
     _exit(1);
-    /* LCOV_EXCL_STOP */
   }
 
   /* We are in the parent process. */

src/lib/process/process_win32.c

@@ -234,6 +234,24 @@ process_win32_exec(process_t *process)
     CloseHandle(stdin_pipe_read);
     CloseHandle(stdin_pipe_write);
 
+    /* In the Unix backend, we do not get an error in the Tor process when a
+     * child process fails to spawn its target executable since we need to
+     * first do the fork() call in the Tor process and then the child process
+     * is responsible for doing the call to execve().
+     *
+     * This means that the user of the process_exec() API must check for
+     * whether it returns PROCESS_STATUS_ERROR, which will rarely happen on
+     * Unix, but will happen for error cases on Windows where it does not
+     * happen on Unix. For example: when the target executable does not exist
+     * on the file system.
+     *
+     * To have somewhat feature compatibility between the Unix and the Windows
+     * backend, we here notify the process_t owner that the process have exited
+     * (even though it never managed to run) to ensure that the exit callback
+     * is executed.
+     */
+    process_notify_event_exit(process, 0);
+
     return PROCESS_STATUS_ERROR;
   }
 

src/test/test_process_slow.c

@@ -328,8 +328,38 @@ test_callbacks_terminate(void *arg)
   process_free(process);
 }
 
+static void
+test_nonexistent_executable(void *arg)
+{
+  (void)arg;
+
+  /* Process callback data. */
+  process_data_t *process_data = process_data_new();
+
+  /* Setup our process. */
+  process_t *process = process_new("binary-does-not-exist");
+  process_set_data(process, process_data);
+  process_set_exit_callback(process, process_exit_callback);
+
+  /* Run our process. */
+  process_exec(process);
+
+  /* Start our main loop. */
+  run_main_loop(process_data);
+
+  /* Ensure that the exit callback was actually called even though the binary
+   * did not exist.
+   */
+  tt_assert(process_data->did_exit);
+
+ done:
+  process_data_free(process_data);
+  process_free(process);
+}
+
 struct testcase_t slow_process_tests[] = {
   { "callbacks", test_callbacks, 0, NULL, NULL },
   { "callbacks_terminate", test_callbacks_terminate, 0, NULL, NULL },
+  { "nonexistent_executable", test_nonexistent_executable, 0, NULL, NULL },
   END_OF_TESTCASES
 };