[PATCH xorg-gtest 2/2] process: add a timeout wait to Kill and Terminate

[Chase Douglas]

On 07/11/2012 05:25 PM, Peter Hutterer wrote:
> Default of 0 has the same behaviour, but for any other timeout, we wait up
> to that timeout and then return.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> I think this is the better implementation over having a separate function
> call that the caller needs to invoke. Just calling Kill(1000) to wait a
> second for the process to die.

Sounds good to me. This is an interesting case where the change is an 
ABI break, but not an API break. However, since you have to compile 
xorg-gtest anew every time you use it, we should be safe :).

I think in the future we might end up exposing WaitForExit(). It's a 
handy method if you are actually expecting a process to exit. We can 
cross that bridge when we get there, though.

Reviewed-by: Chase Douglas <chase.douglas at canonical.com>

>
>   include/xorg/gtest/xorg-gtest-process.h |   27 ++++++++++++++++++++-------
>   src/process.cpp                         |   29 +++++++++++++++++++++++------
>   2 files changed, 43 insertions(+), 13 deletions(-)
>
> diff --git a/include/xorg/gtest/xorg-gtest-process.h b/include/xorg/gtest/xorg-gtest-process.h
> index e8c8d14..ca853d4 100644
> --- a/include/xorg/gtest/xorg-gtest-process.h
> +++ b/include/xorg/gtest/xorg-gtest-process.h
> @@ -137,28 +137,40 @@ class Process {
>     void Start(const std::string& program, ...);
>
>     /**
> -   * Terminates (SIGTERM) this child process.
> +   * Terminates (SIGTERM) this child process and waits a given timeout for
> +   * the process to terminate.
> +   *
> +   * @param [in] timeout The timeout in millis to wait for the process to
> +   *                     terminate. A timeout of 0 implies not to wait but
> +   *                     return immediately.
>      *
>      * @throws std::runtime_error if child tries to terminate itself.
>      *
> -   * @returns true if termination succeeded, false otherwise.
> +   * @returns true if termination succeeded and, if a timout is given, the
> +   *          process shut down within that timeout. false otherwise.
>      *
>      * @post If successful: Child process terminated.
>      * @post If successful: Subsequent calls to Pid() return -1.
>      */
> -  bool Terminate();
> +  bool Terminate(unsigned int timeout = 0);
>
>     /**
> -   * Kills (SIGKILL) this child process.
> +   * Kills (SIGKILL) this child process and waits a given timeout for the
> +   * process to terminate.
> +   *
> +   * @param [in] timeout The timeout in millis to wait for the process to
> +   *                     terminate. A timeout of 0 implies not to wait but
> +   *                     return immediately.
>      *
>      * @throws std::runtime_error if child tries to kill itself.
>      *
> -   * @returns true if kill succeeded, false otherwise.
> +   * @returns true if kill succeeded and, if a timout is given, the
> +   *          process shut down within that timeout. false otherwise.
>      *
>      * @post If successful: Child process killed.
>      * @post If successful: Subsequent calls to Pid() return -1.
>      */
> -  bool Kill();
> +  bool Kill(unsigned int timeout = 0);
>
>     /**
>      * Accesses the pid of the child process.
> @@ -174,7 +186,8 @@ class Process {
>     /* Disable copy constructor, assignment operator */
>     Process(const Process&);
>     Process& operator=(const Process&);
> -  bool KillSelf(int signal);
> +  bool WaitForExit(unsigned int timeout);
> +  bool KillSelf(int signal, unsigned int timout);
>   };
>
>   } // testing
> diff --git a/src/process.cpp b/src/process.cpp
> index be3bbda..7138011 100644
> --- a/src/process.cpp
> +++ b/src/process.cpp
> @@ -93,8 +93,23 @@ void xorg::testing::Process::Start(const std::string& program, ...) {
>     va_end(list); /* Shouldn't get here */
>   }
>
> +bool xorg::testing::Process::WaitForExit(unsigned int timeout) {
> +  for (int i = 0; i < 10; i++) {
> +    int status;
> +    int pid = waitpid(Pid(), &status, WNOHANG);
> +
> +    if (pid == Pid())
> +      return true;
> +
> +      usleep(timeout * 100);
> +  }
> +
> +  return false;
> +}
> +
> +bool xorg::testing::Process::KillSelf(int signal, unsigned int timeout) {
> +  bool wait_success = true;
>
> -bool xorg::testing::Process::KillSelf(int signal) {
>     if (d_->pid == -1) {
>       return false;
>     } else if (d_->pid == 0) {
> @@ -105,17 +120,19 @@ bool xorg::testing::Process::KillSelf(int signal) {
>         d_->pid = -1;
>         return false;
>       }
> +    if (timeout > 0)
> +      wait_success = WaitForExit(timeout);
>       d_->pid = -1;
>     }
> -  return true;
> +  return wait_success;
>   }
>
> -bool xorg::testing::Process::Terminate(void) {
> -  return KillSelf(SIGTERM);
> +bool xorg::testing::Process::Terminate(unsigned int timeout) {
> +  return KillSelf(SIGTERM, timeout);
>   }
>
> -bool xorg::testing::Process::Kill(void) {
> -  return KillSelf(SIGKILL);
> +bool xorg::testing::Process::Kill(unsigned int timeout) {
> +  return KillSelf(SIGKILL, timeout);
>   }
>
>   void xorg::testing::Process::SetEnv(const std::string& name,
>