arun11299 / cpp-subprocess

Subprocessing with modern C++
Other
449 stars 90 forks source link

Popen blocking on Windows #75

Closed philippewarren closed 2 years ago

philippewarren commented 2 years ago

On Windows, the call to std::async in execute_process blocks until the process dies naturally, which is problematic if the process does not die and the intended use case is to kill it ourselves. Could be easily fixed by storing the return value (of type std::future) of the call to std::async as an instance member. This way, the wait on the future would only happen when the instance is destroyed. This:

  this->cleanup_future_ = std::async(std::launch::async, [this] {
    WaitForSingleObject(this->process_handle_, INFINITE);

    CloseHandle(this->stream_.g_hChildStd_ERR_Wr);
    CloseHandle(this->stream_.g_hChildStd_OUT_Wr);
    CloseHandle(this->stream_.g_hChildStd_IN_Rd);
  });

instead of this:

  std::async(std::launch::async, [this] {
    WaitForSingleObject(this->process_handle_, INFINITE);

    CloseHandle(this->stream_.g_hChildStd_ERR_Wr);
    CloseHandle(this->stream_.g_hChildStd_OUT_Wr);
    CloseHandle(this->stream_.g_hChildStd_IN_Rd);
  });
HUMANIAM commented 2 years ago

@philippewarren I faced the same issue on Windows when trying to create std::Popen with subprocess::Popen(command, sp::output{sp::PIPE});. I tried your suggested solution and it worked fine but what I don't understand is

philippewarren commented 2 years ago

If the std:: future is not bound, it is destroyed, and its destructor joins on the async thread, which is why it blocks. Source: https://en.cppreference.com/w/cpp/thread/async