From f56f1b05c0ddb492bdf3b3d885318351f4c0ed7a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 13 Jun 2016 09:15:20 -0700
Subject: [PATCH] Install a SIGTERM handler in ssh-connect

Summary:
Ref T10547. This has been around for a while but I was never able to reproduce it. I caught a repro case in the cluster recently and I think this is the right fix.

We tell Subversion to run `ssh-connect` instead of `ssh` so we can provide options and credentials, by using `SVN_SSH` in the environment. Subversion will sometimes kill the SSH tunnel subprocess aggressively with SIGTERM -- as of writing, you can search for `SIGTERM` in `make_tunnel()` here:

http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/client.c

By default, when a PHP process gets SIGTERM it just exits immediately, without running destructors or shutdown functions. Since destructors/shutdown functions don't run, `TempFile` doesn't get a chance to remove the file.

I don't have a clear picture of //when// Subversion sends SIGTERM to the child process. I can't really get this to trigger locally via `svn`, although I was able to get it to trigger explicitly. So I'm only about 95% sure this fixes it, but it seems likely.

Test Plan:
Locally, I couldn't get this to reproduce "normally" even knowing the cause (maybe Subversion doesn't do the SIGTERM stuff on OSX?) but I was able to get it to reproduce reliabily by adding `posix_kill(getmypid(), SIGTERM);` to the body of the script.

With that added, running the script with `PHABRICATOR_CREDENTIAL=PHID-CDTL-...` in the environment reliably left straggler temporary files.

Adding `declare()` and a signal handler fixed this: the script now runs the `TempFile` destructor and longer leaves the stragglers around.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10547

Differential Revision: https://secure.phabricator.com/D16102
---
 scripts/ssh/ssh-connect.php | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/scripts/ssh/ssh-connect.php b/scripts/ssh/ssh-connect.php
index fac8d19130..d42a542140 100755
--- a/scripts/ssh/ssh-connect.php
+++ b/scripts/ssh/ssh-connect.php
@@ -4,6 +4,11 @@
 // This is a wrapper script for Git, Mercurial, and Subversion. It primarily
 // serves to inject "-o StrictHostKeyChecking=no" into the SSH arguments.
 
+// In some cases, Subversion sends us SIGTERM. If we don't catch the signal and
+// react to it, we won't run object destructors by default and thus won't clean
+// up temporary files. Declare ticks so we can install a signal handler.
+declare(ticks=1);
+
 $root = dirname(dirname(dirname(__FILE__)));
 require_once $root.'/scripts/__init_script__.php';
 
@@ -21,6 +26,16 @@ $args->parsePartial(
   ));
 $unconsumed_argv = $args->getUnconsumedArgumentVector();
 
+if (function_exists('pcntl_signal')) {
+  pcntl_signal(SIGTERM, 'ssh_connect_signal');
+}
+
+function ssh_connect_signal($signo) {
+  // This is just letting destructors fire. In particular, we want to clean
+  // up any temporary files we wrote. See T10547.
+  exit(128 + $signo);
+}
+
 $pattern = array();
 $arguments = array();