Page MenuHomePhabricator

Temporary files are not removed when PHP double-fatals (e.g., method call on NULL)
Closed, ResolvedPublic


I've noticed that I've been experiencing a rather unusual buildup of Passphrase temporary files building up in my /tmp filesystem - about 1.3 million to be precise. Some initial investigation suggested by @epriestley, namely P1040 (the test tmp file was deleted on my system) and P1041 (did not stop the behavior).

Starting the phd daemons in verbose/trace mode yielded no apparently useful debug.

After randomly flipping switches like a rabid monkey, I have observed that disabling tracking of an externally hosted subversion repository appears to have cause this behavior to cease in my Phabricator install.

Event Timeline

wotte raised the priority of this task from to Needs Triage.
wotte updated the task description. (Show Details)
wotte added projects: Diffusion, Passphrase.
wotte added a subscriber: wotte.

Thanks, epriestley. Is this still being looked into as a potential bug? Disabling externally hosted subversion repos isnt really a viable solution for most people, I wouldnt think.

This is definitely a bug, but one we haven't been able to reproduce yet.

Could any of the settings at the page below pertain to this?


Mine are currently:
Number of seconds to retain Herald transcripts for.
Current Value: 2592000
Number of seconds to retain Daemon logs for.
Current Value: 604800
Number of seconds to retain Differential parse caches for.
Current Value: 1209600
Number of seconds to retain Markup cache entries for.
Current Value: 2592000
Number of seconds to retain archived background tasks for.
Current Value: 1209600
Number of seconds to retain general cache entries for.
Current Value: 2592000
Number of seconds to retain Conduit call logs for.
Current Value: 15552000

No, those are all unrelated.

The expectation is that bin/ssh-connect will write these files, use them to execute an ssh command, then clean them up when it exits. This should be the only way these files are written, and they should automatically be destroyed when the process exits. On my machine, this appears to happen. From @wrotte's tests, the cleanup mechanism seems to work properly (P1040) but even explicit destruction of the keyfile did not (P1041). I don't have a plausible explanation for this.

You can reproduce the circumstances which are expected to create the key like this:

$ PHABRICATOR_CREDENTIAL=PHID-CDTL-xxxx SVN_SSH=/path/to/phabricator/bin/ssh-connect svn --non-interactive info ssh://


  • PHID-CDTL-xxxx is the PHID of the Passphrase credential with the key. This isn't easy to obtain, but you can query the phabricator_passphrase database.
  • /path/to/phabricator/bin/ssh-connect is the path to bin/ssh-connect in Phabricator.
  • ssh:// is the SVN remote URI.

Under the hood, this will run svn, which will invoke ssh-connect in order to run the underlying ssh process. ssh-connect will read the credential identifier from the environment, load the credential, write it to disk, and then invoke an ssh -l <user> -i <key> <uri> command to establish the connection.

When ssh-connect exits, it is supposed to clean up the temporary file.

I might investigate a bit more in a bit, I've just been slammed lately and haven't had much available time to put towards the care and feeding of my phab install.

No worries! I'll look into this some more too when I have a chance.

It could be related:

Steps to reproduce:

  1. Add a credential for a remote repository. (SSH Key)
  2. Destroy the credential.
  3. Voila!

The destroyed credential keeps attached to the remote uri. An uncaught exception raises so the temp file is never deleted:

Call to a member function openEnvelope() on null in .../phabricator/src/applications/passphrase/keys/PassphraseSSHKey.php on line 29

getSecret() returns null:

if (!$this->keyFile) {
  $temporary_file = new TempFile('passphrase-ssh-key');

  Filesystem::changePermissions($temporary_file, 0600);


  $this->keyFile = $temporary_file;

Hope it helps!

That's extremely helpful, thanks! I'll get that fixed up.

Although I would expect the temporary file to be destroyed even if the exception is raised. This is still a bug in any case, since we shouldn't write the file in the first place.

Oh, except that when you call a method on null we don't actually get an exception because PHP is sort of derp and everything just dies abruptly. This script reproduces the issue, at least potentially:


require_once 'scripts/__init_script__.php';

function write_file($fatal) {
  $f = new TempFile();
  echo "Writing file: {$f}\n";

  if ($fatal) {
    echo "Fataling!\n";
    Filesystem::writeFile($f, fatal_now());
  } else {
    echo "Throwing!\n";
    Filesystem::writeFile($f, throw_exception());

function throw_exception() {
  throw new Exception("X");

function fatal_now() {
  $o = null;


When run as:

$ php -f test.php

...we throw, which destroys the temporary file.

However, when run as:

$ php -f test.php 1

...we fatal by calling a method on null, which does not.

@wotte / @wassere, if you still see this locally, can you check if that fixed your issue?

I'm going to tentatively assume that it's either directly connected to that bug or caused by a similar issue and redirect this to finding ways to clean up temporary files more aggressively. It's bad that we don't destroy them when we hit multiple fatals.

epriestley renamed this task from Polling remotely hosted subversion repositories doesn't appear to garbage collect the Passphrase tmp file holding the SSH key. to Temporary files are not removed when PHP double-fatals (e.g., method call on NULL).Aug 21 2014, 6:29 PM

I'm still working through my backlog after a long vacation to Iceland, but I'll nuke the cron job I had cleaning out the test directory and see if we still see the behavior.

Cool. A possible first-degree approximation is checking if the files contain private key material -- I'm not sure if we ever did that. If they're empty, that's a smoking gun for this being the issue.

I think D10329 will resolve this regardless, though, hopefully.

Thats awesome, thanks guys! I had a cron job deleting these file hourly as well. Please let us know if the issue is indeed fixed with Evan's commit.

I'm still getting this problem with the latest pull :(
A few hundred thousand copies of my key every week :(

Not sure exactly where to start looking into it, or things to try to fix it?

Edit: I have 6 externally hosted repos running in my phabricator-install and updates appear correctly inside phabricator, indicating that the operation itself seems to work fine, even though the temporary key file is left in /tmp/ afterwards. The repos are accessed over svn+ssh with a shared, stored key.

I'm seeing this on the current version in my environment also. As oyvindselbek mentioned, everything seems to be working (updates are happening) except for the cleanup of the temp files.

I started troubleshooting this a bit, but I'm hung up at the proc_open call in PhutilExecPassThru. It seems like neither the error handler nor output ever get restored after a successful call to proc_open, so any items logged or exceptions thrown never show up after that point. I'm not sure if this is expected behavior or a bug, but it sure makes troubleshooting difficult.

I expect that there's an error occurring with the deletion, but we never see it because of the redirection.

Anyway, I can reproduce this at will in our environment, so if I can provide any additional info, run test cases, etc., please let me know.