Francois Gouget : testbot/testagentd: Add a RemoveChildProc RPC.

Alexandre Julliard julliard at wine.codeweavers.com
Wed Jul 23 13:49:23 CDT 2014


Module: tools
Branch: master
Commit: 2d08b161f0244f05219dcee936491e140117ec3d
URL:    http://source.winehq.org/git/tools.git/?a=commit;h=2d08b161f0244f05219dcee936491e140117ec3d

Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Mon Jul 21 23:24:00 2014 +0200

testbot/testagentd: Add a RemoveChildProc RPC.

The Wait RPC no longer causes the server to forget about the child
process. That reponsibility is transferred to the RemoveChildProc
RPC. This is to solve the case where the network connection breaks at
just the time when the child process terminates which could cause the
server to forget the child process exit status despite it not having
reached the client process.

---

 testbot/lib/WineTestBot/TestAgent.pm      | 26 ++++++++++++++++++++++++++
 testbot/scripts/TestAgent                 | 12 ++++++++++--
 testbot/src/testagentd/platform.h         |  8 ++++++++
 testbot/src/testagentd/platform_unix.c    | 17 +++++++++++++++++
 testbot/src/testagentd/platform_windows.c | 27 ++++++++++++++++++++++++---
 testbot/src/testagentd/testagentd.c       | 25 ++++++++++++++++++++++++-
 6 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/testbot/lib/WineTestBot/TestAgent.pm b/testbot/lib/WineTestBot/TestAgent.pm
index 1acb0cd..2cb61ff 100644
--- a/testbot/lib/WineTestBot/TestAgent.pm
+++ b/testbot/lib/WineTestBot/TestAgent.pm
@@ -39,6 +39,7 @@ my $RPC_WAIT2 = 6;
 my $RPC_SETTIME = 7;
 my $RPC_GETPROPERTIES = 8;
 my $RPC_UPGRADE = 9;
+my $RPC_RMCHILDPROC = 10;
 
 my %RpcNames=(
     $RPC_PING => 'ping',
@@ -51,6 +52,7 @@ my %RpcNames=(
     $RPC_SETTIME => 'settime',
     $RPC_GETPROPERTIES => 'getproperties',
     $RPC_UPGRADE => 'upgrade',
+    $RPC_RMCHILDPROC => 'rmchildproc',
 );
 
 my $Debug = 0;
@@ -1381,4 +1383,28 @@ sub Upgrade($$)
   return $rc;
 }
 
+sub RemoveChildProcess($$)
+{
+  my ($self, $Pid) = @_;
+  debug("RmChildProcess $Pid\n");
+
+  # Make sure we have the server version
+  return undef if (!$self->{agentversion} and !$self->_Connect());
+
+  # Up to 1.5 a seemingly successful Wait RPC automatically removes child
+  # processes.
+  return 1 if ($self->{agentversion} =~ / 1\.[0-5]$/);
+
+  # Send the command
+  if (!$self->_StartRPC($RPC_RMCHILDPROC) or
+      !$self->_SendListSize('ArgC', 1) or
+      !$self->_SendUInt64('Pid', $Pid))
+  {
+      return undef;
+  }
+
+  # Get the reply
+  return $self->_RecvList('');
+}
+
 1;
diff --git a/testbot/scripts/TestAgent b/testbot/scripts/TestAgent
index ac2c914..ec4504f 100755
--- a/testbot/scripts/TestAgent
+++ b/testbot/scripts/TestAgent
@@ -376,14 +376,22 @@ elsif ($Cmd eq "run")
         if (!($RunFlags & $TestAgent::RUN_DNT))
         {
             $Result = $TA->Wait($Pid, $Timeout, $Keepalive);
-            print "Child exit status: $Result\n" if (defined $Result);
+            if (defined $Result)
+            {
+                print "Child exit status: $Result\n";
+                $TA->RemoveChildProcess($Pid);
+            }
         }
     }
 }
 elsif ($Cmd eq "wait")
 {
     $Result = $TA->Wait($WaitPid, $Timeout, $Keepalive);
-    print "Child exit status: $Result\n" if (defined $Result);
+    if (defined $Result)
+    {
+        print "Child exit status: $Result\n";
+        $TA->RemoveChildProcess($WaitPid);
+    }
 }
 elsif ($Cmd eq "rm")
 {
diff --git a/testbot/src/testagentd/platform.h b/testbot/src/testagentd/platform.h
index c6c0f2e..46e4d37 100644
--- a/testbot/src/testagentd/platform.h
+++ b/testbot/src/testagentd/platform.h
@@ -74,11 +74,19 @@ uint64_t platform_run(char** argv, uint32_t flags, char** redirects);
 /* If a command was started in the background, waits until either that command
  * terminates, the specified timeout (in seconds) expires, or the client
  * disconnects (typically because it got tired of waiting).
+ * Note that this does not cause the child process to be forgotten, even if it
+ * did exit. This is so that the client can retrieve the information again if
+ * needed (e.g. in case it did not receive it due to a network issue).
  * If no command was started in the background, then reports an error
  * immediately.
  */
 int platform_wait(SOCKET client, uint64_t pid, uint32_t timeout, uint32_t *childstatus);
 
+/* Causes the given child process to be forgotten, which means it will no longer
+ * be possible to wait for it or retrieve its exit status.
+ */
+int platform_rmchildproc(SOCKET client, uint64_t pid);
+
 /* Sets the system time to the specified Unix epoch. If the system time is
  * already within leeway seconds of the specified time, then consider that
  * the system clock is already correct.
diff --git a/testbot/src/testagentd/platform_unix.c b/testbot/src/testagentd/platform_unix.c
index 75afc81..b27c016 100644
--- a/testbot/src/testagentd/platform_unix.c
+++ b/testbot/src/testagentd/platform_unix.c
@@ -194,6 +194,23 @@ int platform_wait(SOCKET client, uint64_t pid, uint32_t timeout, uint32_t *child
     }
     debug("process " U64FMT " returned status %u\n", pid, child->status);
     *childstatus = child->status;
+    return 1;
+}
+
+int platform_rmchildproc(SOCKET client, uint64_t pid)
+{
+    struct child_t *child;
+
+    LIST_FOR_EACH_ENTRY(child, &children, struct child_t, entry)
+    {
+        if (child->pid == pid)
+            break;
+    }
+    if (!child || child->pid != pid)
+    {
+        set_status(ST_ERROR, "the " U64FMT " process does not exist or is not a child process", pid);
+        return 0;
+    }
     list_remove(&child->entry);
     free(child);
     return 1;
diff --git a/testbot/src/testagentd/platform_windows.c b/testbot/src/testagentd/platform_windows.c
index dd8da8e..8e00f09 100644
--- a/testbot/src/testagentd/platform_windows.c
+++ b/testbot/src/testagentd/platform_windows.c
@@ -204,9 +204,9 @@ int platform_wait(SOCKET client, uint64_t pid, uint32_t timeout, uint32_t *child
         debug("WaitForMultipleObjects() returned %lu (le=%lu). Giving up!\n", r, GetLastError());
         break;
     }
-    CloseHandle(child->handle);
-    list_remove(&child->entry);
-    free(child);
+    /* Don't close child->handle so we can retrieve the exit status again if
+     * needed.
+     */
 
  cleanup:
     /* We must reset WSAEventSelect before we can make
@@ -221,6 +221,27 @@ int platform_wait(SOCKET client, uint64_t pid, uint32_t timeout, uint32_t *child
     return success;
 }
 
+int platform_rmchildproc(SOCKET client, uint64_t pid)
+{
+    struct child_t *child;
+
+    LIST_FOR_EACH_ENTRY(child, &children, struct child_t, entry)
+    {
+        if (child->pid == pid)
+            break;
+    }
+    if (!child || child->pid != pid)
+    {
+        set_status(ST_ERROR, "the " U64FMT " process does not exist or is not a child process", pid);
+        return 0;
+    }
+
+    CloseHandle(child->handle);
+    list_remove(&child->entry);
+    free(child);
+    return 1;
+}
+
 int platform_settime(uint64_t epoch, uint32_t leeway)
 {
     FILETIME filetime;
diff --git a/testbot/src/testagentd/testagentd.c b/testbot/src/testagentd/testagentd.c
index 292b1c7..dc181a6 100644
--- a/testbot/src/testagentd/testagentd.c
+++ b/testbot/src/testagentd/testagentd.c
@@ -37,8 +37,9 @@
  * 1.3:  Fix the zero / infinite timeouts in the wait2 RPC.
  * 1.4:  Add the settime RPC.
  * 1.5:  Add support for upgrading the server.
+ * 1.6:  Add support for the rmchildproc RPC.
  */
-#define PROTOCOL_VERSION "testagentd 1.5"
+#define PROTOCOL_VERSION "testagentd 1.6"
 
 #define BLOCK_SIZE       65536
 
@@ -88,6 +89,7 @@ enum rpc_ids_t
     RPCID_SETTIME,
     RPCID_GETPROPERTIES,
     RPCID_UPGRADE,
+    RPCID_RMCHILDPROC,
 };
 
 /* This is the RPC currently being processed */
@@ -108,6 +110,7 @@ static const char* rpc_name(uint32_t id)
         "settime",
         "getproperties",
         "upgrade",
+        "rmchildproc",
     };
 
     if (id < sizeof(names) / sizeof(*names))
@@ -837,6 +840,23 @@ static void do_wait2(SOCKET client)
         send_error(client);
 }
 
+static void do_rmchildproc(SOCKET client)
+{
+    uint64_t pid;
+
+    if (!expect_list_size(client, 1) ||
+        !recv_uint64(client, &pid))
+    {
+        send_error(client);
+        return;
+    }
+
+    if (platform_rmchildproc(client, pid))
+        send_list_size(client, 0);
+    else
+        send_error(client);
+}
+
 static void do_rm(SOCKET client)
 {
     int got_errors;
@@ -1090,6 +1110,9 @@ static void process_rpc(SOCKET client)
     case RPCID_UPGRADE:
         do_upgrade(client);
         break;
+    case RPCID_RMCHILDPROC:
+        do_rmchildproc(client);
+        break;
     default:
         do_unknown(client, rpcid);
     }




More information about the wine-cvs mailing list