trafficserver git commit: TS-3104: fix lockfile logic which decides whether to kill process or group

[jp...@apache.org]

Repository: trafficserver
Updated Branches:
  refs/heads/master f2e7c32e1 -> ba0306c35


TS-3104: fix lockfile logic which decides whether to kill process or group


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ba0306c3
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ba0306c3
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ba0306c3

Branch: refs/heads/master
Commit: ba0306c356ad4ec58c8ff77f120c61eaa229c6c9
Parents: f2e7c32
Author: Victor Leschuk <le...@ashmanov.com>
Authored: Thu Jun 11 20:43:28 2015 -0700
Committer: James Peach <jp...@apache.org>
Committed: Thu Jun 11 21:27:12 2015 -0700

----------------------------------------------------------------------
 CHANGES                        |  3 +++
 cmd/traffic_cop/traffic_cop.cc |  8 +++++++-
 lib/ts/lockfile.cc             | 14 +++++++++++---
 3 files changed, 21 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ba0306c3/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 9cc3dfd..dd50d24 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 6.0.0
 
+  *) [TS-3104] Fix lockfile logic which decides whether to kill process or group.
+   Author: Victor Leschuk <le...@ashmanov.com>
+
   *) [TS-3569] Replace UNSAFE_FORCE_MUTEX with an sdk_assert, and relax mutex
    requirements on Schedule(). Also rename MUTEX_LOCK to SCOPED_MUTEX_LOCK, to
    be more descriptive.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ba0306c3/cmd/traffic_cop/traffic_cop.cc
----------------------------------------------------------------------
diff --git a/cmd/traffic_cop/traffic_cop.cc b/cmd/traffic_cop/traffic_cop.cc
index 7adf89f..94ccdb4 100644
--- a/cmd/traffic_cop/traffic_cop.cc
+++ b/cmd/traffic_cop/traffic_cop.cc
@@ -132,6 +132,9 @@ static int child_pid = 0;
 static int child_status = 0;
 static int sem_id = 11452;
 
+// manager API is initialized
+static bool mgmt_init = false;
+
 AppVersionInfo appVersionInfo;
 
 static char const localhost[] = "127.0.0.1";
@@ -1204,6 +1207,7 @@ test_mgmt_cli_port()
 
   if (TSRecordGetString("proxy.config.manager_binary", &val) != TS_ERR_OKAY) {
     cop_log(COP_WARNING, "(cli test) unable to retrieve manager_binary\n");
+    mgmt_init = false;
     ret = -1;
   } else {
     if (strcmp(val, manager_binary) != 0) {
@@ -1606,7 +1610,6 @@ check_no_run()
 static void *
 check(void *arg)
 {
-  bool mgmt_init = false;
   cop_log_trace("Entering check()\n");
 
   for (;;) {
@@ -1655,7 +1658,10 @@ check(void *arg)
     // We do this after the first round of checks, since the first "check" will spawn traffic_manager
     if (!mgmt_init) {
       ats_scoped_str runtimedir(config_read_runtime_dir());
+
+      cop_log(COP_DEBUG, "initializing manager API\n");
       TSInit(runtimedir, static_cast<TSInitOptionT>(TS_MGMT_OPT_NO_EVENTS));
+
       mgmt_init = true;
 
       // Allow a configurable longer sleep init time

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ba0306c3/lib/ts/lockfile.cc
----------------------------------------------------------------------
diff --git a/lib/ts/lockfile.cc b/lib/ts/lockfile.cc
index d8646ad..7ca9b83 100644
--- a/lib/ts/lockfile.cc
+++ b/lib/ts/lockfile.cc
@@ -241,6 +241,7 @@ Lockfile::KillGroup(int sig, int initial_sig, const char *pname)
   int err;
   pid_t pid;
   pid_t holding_pid;
+  pid_t self = getpid();
 
   err = Open(&holding_pid);
   if (err == 1) // success getting the lock file
@@ -252,12 +253,19 @@ Lockfile::KillGroup(int sig, int initial_sig, const char *pname)
       pid = getpgid(holding_pid);
     } while ((pid < 0) && (errno == EINTR));
 
-    if ((pid < 0) || (pid == getpid()))
+    if ((pid < 0) || (pid == self)) {
+      // Error getting process group,
+      // or we are the group's owner.
+      // Let's kill just holding_pid
       pid = holding_pid;
-
-    if (pid != 0) {
+    } else if (pid != self) {
+      // We managed to get holding_pid's process group
+      // and this group is not ours.
       // This way, we kill the process_group:
       pid = -pid;
+    }
+
+    if (pid != 0) {
       // In order to get core files from each process, please
       // set your core_pattern appropriately.
       lockfile_kill_internal(holding_pid, initial_sig, pid, pname, sig);