DaanDeMeyer / reproc

A cross-platform (C99/C++11) process library
MIT License
552 stars 65 forks source link

Assertion failed: status <= 0x7fffffff #45

Closed alex-tee closed 3 years ago

alex-tee commented 4 years ago

My unit tests on travis are failing with the following line on windows (msys):

Assertion failed: status <= 0x7fffffff, file ../subprojects/reproc/reproc/src/process.windows.c, line 470

https://travis-ci.org/github/zrythm/zrythm/jobs/727719758

this is what my test code is doing:

  char * output;
  int ret;

  const char * args[] = {
    "bash", "-c", "sleep 6 && echo hello", NULL, };
  const char * args_stderr[] = {
    "bash", "-c", "sleep 6 && >&2 echo hello", NULL, };
  ret =
    system_run_cmd_w_args (
      args, 1, true, &output, false);
  g_assert_cmpint (ret, !=, 0);
  ret =
    system_run_cmd_w_args (
      args, 1, false, &output, false);
  g_assert_cmpint (ret, !=, 0);
  ret =
    system_run_cmd_w_args (
      args_stderr, 1, true, &output, false);
  g_assert_cmpint (ret, !=, 0);
  ret =
    system_run_cmd_w_args (
      args_stderr, 1, false, &output, false);
  g_assert_cmpint (ret, !=, 0);

  ret =
    system_run_cmd_w_args (
      args, 2000, true, &output, false);
  g_assert_cmpint (ret, !=, 0);
  ret =
    system_run_cmd_w_args (
      args, 2000, false, &output, false);
  g_assert_cmpint (ret, !=, 0);
  ret =
    system_run_cmd_w_args (
      args_stderr, 2000, true, &output, false);
  g_assert_cmpint (ret, !=, 0);
  ret =
    system_run_cmd_w_args (
      args_stderr, 2000, false, &output, false);
  g_assert_cmpint (ret, !=, 0);

here is the function I'm testing:

/**
 * Runs the given command in the background, waits
 * for it to finish and returns its exit code.
 *
 * @note Only works for stdout for now.
 *
 * @param args NULL-terminated array of args.
 * @param get_stdout Whether to get the standard out
 *   (true) or stderr (false).
 * @param[out] output A pointer to save the newly
 *   allocated stdout or stderr output.
 * @param ms_timer A timer in ms to
 *   kill the process, or negative to not
 *   wait.
 */
int
system_run_cmd_w_args (
  const char ** args,
  int           ms_to_wait,
  bool          get_stdout,
  char **       output,
  bool          warn_if_fail)
{
  g_message ("ms to wait: %d", ms_to_wait);

  *output = NULL;

  reproc_options opts;
  memset (&opts, 0, sizeof (reproc_options));
  opts.stop.first.action = REPROC_STOP_WAIT;
  opts.stop.first.timeout = REPROC_DEADLINE;
  opts.stop.second.action = REPROC_STOP_TERMINATE;
  opts.stop.second.timeout = 1000;
  opts.stop.third.action = REPROC_STOP_KILL;
  opts.stop.third.timeout = REPROC_INFINITE;
  opts.deadline = ms_to_wait;

  reproc_sink sink = reproc_sink_string (output);
  int r =
    reproc_run_ex (
      args, opts,
      get_stdout ? sink : REPROC_SINK_NULL,
      get_stdout ? REPROC_SINK_NULL : sink);

  g_message ("output: %s", *output);

  if (r < 0)
    {
      if (warn_if_fail)
        {
          g_warning ("%s", reproc_strerror (r));
        }
      else
        {
          g_message ("%s", reproc_strerror (r));
        }
    }

  return (r < 0) ? r : 0;
}

any idea what this means?

DaanDeMeyer commented 4 years ago

reproc assumes that the exit code of processes on Windows is smaller than INT_MAX. I guess bash is returning exit codes outside of that range. Maybe look into why bash is returning such a large exit code. We might have to change reproc to deal with exit codes outside of the range of an int to fix this in reproc itself.

tafuri commented 3 years ago

A similar problem occurs on windows when a process returns a negative value (for example Clang). Since GetExitCodeProcess will already retrieve an unsigned DWORD, the assert ASSERT(status <= INT_MAX) does not really make sense.

DaanDeMeyer commented 3 years ago

You're actually right, I was worried about overflowing the integer but the DWORD is really just an int under the covers since main() returns an int. I wonder if returning negative values is really best practice though. Regardless, I'll remove the assert shortly.

DaanDeMeyer commented 3 years ago

Fixed by removing the assert. Will release a new version sometime soon.