[PATCH xorg-gtest 06/16] xserver: move Terminate and Kill handling here

[Peter Hutterer]

On Fri, Jul 06, 2012 at 11:08:13AM -0700, Chase Douglas wrote:
> On 07/05/2012 05:50 PM, Peter Hutterer wrote:
> >On Thu, Jul 05, 2012 at 02:09:13PM -0700, Chase Douglas wrote:
> >>On 07/03/2012 10:42 PM, Peter Hutterer wrote:
> >>>On Tue, Jul 03, 2012 at 10:33:50AM -0700, Chase Douglas wrote:
> >>>>On 07/02/2012 11:44 PM, Peter Hutterer wrote:
> >>>>>Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >>>>>---
> >>>>>  include/xorg/gtest/xorg-gtest-xserver.h |   13 ++++++++++++
> >>>>>  src/environment.cpp                     |   31 +++-------------------------
> >>>>>  src/xserver.cpp                         |   34 +++++++++++++++++++++++++++++++
> >>>>>  3 files changed, 50 insertions(+), 28 deletions(-)
> >>>>>
> >>>>>diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h
> >>>>>index 52a2fd0..821b01f 100644
> >>>>>--- a/include/xorg/gtest/xorg-gtest-xserver.h
> >>>>>+++ b/include/xorg/gtest/xorg-gtest-xserver.h
> >>>>>@@ -53,6 +53,19 @@ class XServer : public xorg::testing::Process {
> >>>>>      void Start(std::string &program);
> >>>>>
> >>>>>      /**
> >>>>>+     * Terminates this server process. Will signal the server to terminate
> >>>>>+     * multiple times before giving up.
> >>>>>+     *
> >>>>>+     * @return false if the server did not terminate, true otherwise
> >>>>>+     */
> >>>>>+    bool Terminate(void);
> >>>>>+
> >>>>>+    /**
> >>>>>+     * Kills the server. With a vengeance.
> >>>>>+     */
> >>>>>+    bool Kill(void);
> >>>>
> >>>>We don't need to recreate these functions. We've already inherited
> >>>>them from xorg::testing::Process. Those implementations should work
> >>>>automatically if we set up the XServer class properly.
> >>>
> >>>The implementation is different to the one in Process, this one does the
> >>>server-specific bits like waiting for the process to shut down and then
> >>>complaining to the log.
> >>
> >>I read too quickly to realize the difference between the
> >>Environment/XServer implementation and the Process implementation,
> >>but I'm wondering if we should just move the extra stuff to the
> >>Process implementation. Then we won't need to do any redefinition or
> >>overriding.
> >
> >I think we should keep Process basic. For most processes failing to kill it
> >should be an issue in itself, it's just the server that takes too long to
> >shut down and needs special handling.
> 
> I thought about that too. I think the question to ask is: do we
> think most users or subclasses of Process are going to want wait for
> the process to really die. The Process::Kill and Terminate methods
> just try once to raise the signal to the child process. They return
> true if the signal was sent, but don't check what happens after
> that. Perhaps a better approach to resolving this issue would be to
> add second versions of these methods that would take a timeout value
> and return true only if the signal was successfully sent and the
> process died.
> KillAndCheck(timeout) and TerminateAndCheck(timeout) maybe?

maybe so, but let's worry about that when we have other users of Process
that need this feature in a generic parent class?

Cheers,
  Peter