Homec4science

Install a SIGTERM handler in ssh-connect

Authored by epriestley <git@epriestley.com> on Jun 13 2016, 18:15.

Description

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

Details

Committed
epriestley <git@epriestley.com>Jun 13 2016, 19:05
Pushed
aubortJan 31 2017, 17:16
Parents
rPHbba53205de4a: Remove all uses of PhutilGitURI in Phabricator
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPHf56f1b05c0dd: Install a SIGTERM handler in ssh-connect (authored by epriestley <git@epriestley.com>).Jun 13 2016, 19:05