Merge branch 'kill-keepalives' into devel

packages/webapp/webapp_server.js

@@ -35,7 +35,11 @@ var bundledJsCssPrefix;
 // pidfiles.
 // XXX This should really be part of the boot script, not the webapp package.
 //     Or we should just get rid of it, and rely on containerization.
-
+//
+// XXX COMPAT WITH 0.9.2.2
+// Keepalives have been replaced with a check that the parent pid is
+// still running. We keep the --keep-alive option for backwards
+// compatibility.
 var initKeepalive = function () {
   var keepaliveCount = 0;
 
@@ -54,6 +58,21 @@ var initKeepalive = function () {
   }, 3000);
 };
 
+// As a replacement to the old keepalives mechanism, check for a running
+// parent every few seconds. Exit if the parent is not running.
+var startCheckForLiveParent = function (parentPid) {
+  if (parentPid) {
+    setInterval(function () {
+      try {
+        process.kill(parentPid, 0);
+      } catch (err) {
+        console.log("Parent process is dead! Exiting.");
+        process.exit(1);
+      }
+    });
+  }
+};
+
 
 var sha1 = function (contents) {
   var hash = crypto.createHash('sha1');
@@ -747,7 +766,18 @@ var runWebAppServer = function () {
     // We used to use the optimist npm package to parse argv here, but it's
     // overkill (and no longer in the dev bundle). Just assume any instance of
     // '--keepalive' is a use of the option.
+    // XXX COMPAT WITH 0.9.2.2
+    // We used to expect keepalives to be written to stdin every few
+    // seconds; now we just check if the parent process is still alive
+    // every few seconds.
     var expectKeepalives = _.contains(argv, '--keepalive');
+    // XXX Saddest argument parsing ever, should we add optimist back to
+    // the dev bundle?
+    var parentPid = null;
+    var parentPidIndex = _.indexOf(argv, "--parent-pid");
+    if (parentPidIndex !== -1) {
+      parentPid = argv[parentPidIndex + 1];
+    }
     WebAppInternals.generateBoilerplate();
 
     // only start listening after all the startup code has run.
@@ -755,7 +785,7 @@ var runWebAppServer = function () {
     var host = process.env.BIND_IP;
     var localIp = host || '0.0.0.0';
     httpServer.listen(localPort, localIp, Meteor.bindEnvironment(function() {
-      if (expectKeepalives)
+      if (expectKeepalives || parentPid)
         console.log("LISTENING"); // must match run-app.js
       var proxyBinding;
 
@@ -810,8 +840,12 @@ var runWebAppServer = function () {
       console.error(e && e.stack);
     }));
 
-    if (expectKeepalives)
+    if (expectKeepalives) {
       initKeepalive();
+    }
+    if (parentPid) {
+      startCheckForLiveParent(parentPid);
+    }
     return 'DAEMON';
   };
 };

tools/run-app.js

@@ -65,7 +65,6 @@ var AppProcess = function (options) {
   self.settings = options.settings;
 
   self.proc = null;
-  self.keepaliveTimer = null;
   self.madeExitCallback = false;
 };
 
@@ -123,17 +122,6 @@ _.extend(AppProcess.prototype, {
     // exception and the whole app dies.
     // http://stackoverflow.com/questions/2893458/uncatchable-errors-in-node-js
     self.proc.stdin.on('error', function () {});
-
-    // Keepalive so child process can detect when we die
-    self.keepaliveTimer = setInterval(function () {
-      try {
-        if (self.proc && self.proc.pid &&
-            self.proc.stdin && self.proc.stdin.write)
-          self.proc.stdin.write('k');
-      } catch (e) {
-        // do nothing. this fails when the process dies.
-      }
-    }, 2000);
   },
 
   _maybeCallOnExit: function (code, signal) {
@@ -156,10 +144,6 @@ _.extend(AppProcess.prototype, {
     }
     self.proc = null;
 
-    if (self.keepaliveTimer)
-      clearInterval(self.keepaliveTimer);
-    self.keepaliveTimer = null;
-
     self.onListen = null;
     self.onExit = null;
   },
@@ -207,7 +191,7 @@ _.extend(AppProcess.prototype, {
       // Old-style bundle
       var opts = _.clone(self.nodeOptions);
       opts.push(path.join(self.bundlePath, 'main.js'));
-      opts.push('--keepalive');
+      opts.push('--parent-pid', process.pid);
 
       return child_process.spawn(process.execPath, opts, {
         env: self._computeEnvironment()

tools/tests/apps/app-prints-pid/.meteor/.gitignore

@@ -0,0 +1 @@
+local

tools/tests/apps/app-prints-pid/.meteor/packages

@@ -0,0 +1,6 @@
+# Meteor packages used by this project, one per line.
+#
+# 'meteor add' and 'meteor remove' will edit this file for you,
+# but you can also edit it by hand.
+
+meteor-platform

tools/tests/apps/app-prints-pid/.meteor/release

@@ -0,0 +1 @@
+none

tools/tests/apps/app-prints-pid/print.js

@@ -0,0 +1,5 @@
+if (Meteor.isServer) {
+  Meteor.startup(function () {
+    console.log("My pid is " + process.pid);
+  });
+}

tools/tests/run.js

@@ -340,3 +340,42 @@ selftest.define("run with mongo crash", ["checkout"], function () {
   run.expectEnd();
   run.expectExit(254);
 });
+
+// Test that when the parent runner process is SIGKILLed, the child
+// process exits also.
+selftest.define("run and SIGKILL parent process", function () {
+  var s = new Sandbox();
+  var run;
+
+  s.createApp("myapp", "app-prints-pid");
+  s.cd("myapp");
+
+  run = s.run();
+  run.waitSecs(10);
+  var match = run.match(/My pid is (\d+)/);
+  var childPid;
+  if (! match || ! match[1]) {
+    selftest.fail("No pid printed");
+  }
+  childPid = match[1];
+
+  process.kill(run.proc.pid, "SIGKILL");
+  // This sleep should be a little more time than the interval at which
+  // the child checks if the parent is still alive, in
+  // packages/webapp/webapp_server.js.
+  utils.sleepMs(3500);
+
+  // Send the child process a signal of 0. If there is no error, it
+  // means that the process is still running, which is not what we
+  // expect.
+  var caughtError;
+  try {
+    process.kill(childPid, 0);
+  } catch (err) {
+    caughtError = err;
+  }
+
+  if (! caughtError) {
+    selftest.fail("Child process " + childPid + " is still running");
+  }
+});