Cleanup fails

[oliversalzburg]

When I use the default parameters, and then call the cleanup callback when I'm done, I get the following error:

  1)  "after all" hook:
     Error: ENOTEMPTY, directory not empty 'C:\Users\Oliver\AppData\Local\Temp\tmp-17520mURkNFVU7XQ6'
      at Object.fs.rmdirSync (fs.js:624:18)
      at C:\Users\Oliver\prj\cli\test\prepare.js:39:3
      at Array.forEach (native)
      at Context.<anonymous> (C:\Users\Oliver\prj\cli\test\prepare.js:38:13)

I'm copying temporary files to that directory, so this wasn't surprising. I then set unsafeCleanup to true and now I get this result:

  1)  "after all" hook:
     Error: ENOENT, no such file or directory 'C:\Users\Oliver\AppData\Local\Temp\tmp-396iEctJyHwBoYu\corePath'
      at Object.fs.readdirSync (fs.js:666:18)
      at C:\Users\Oliver\prj\cli\test\prepare.js:39:3
      at Array.forEach (native)
      at Context.<anonymous> (C:\Users\Oliver\prj\cli\test\prepare.js:38:13)

The weirdest part is, the temporary directory is actually deleted just fine.

[oliversalzburg]

This also doesn't seem to be specific to Windows environments. I'm seeing the same error on our GNU/Linux build system.

[raszi]

Could you provide sample code? Thanks!

[oliversalzburg]

@raszi This is our test environment preparation code, where the problem appears:

var Promise = require( "bluebird" );

var path = require( "path" );
var tmp = require( "tmp" );
var wrench = require( "wrench" );

//noinspection SpellCheckingInspection
var onTeardown = [];

before( function( done ) {
    console.log( "Setting up global test environment…" );

    tmp.setGracefulCleanup();
    tmp.dir( { unsafeCleanup : true }, function onTempDirCreated( error, temporaryPath, cleanupCallback ) {
        console.log( "Running tests in '" + temporaryPath + "'." );

        onTeardown.push( cleanupCallback );

        wrench.copyDirSyncRecursive( "./test/fixtures/environment", temporaryPath, { forceDelete : true } );
        wrench.copyDirSyncRecursive( ".template", path.join( temporaryPath, "instanceConfigTemplatePath" ), { forceDelete : true } );

        var global = require( "./.util/global" );
        global.config = {
            corePath                   : path.join( temporaryPath, "corePath" ),
            instanceConfigRoot         : path.join( temporaryPath, "instanceConfigRoot" ),
            instanceConfigEnabledRoot  : path.join( temporaryPath, "instanceConfigEnabledRoot" ),
            instanceConfigTemplatePath : path.join( temporaryPath, "instanceConfigTemplatePath" ),
            logRootPath                : path.join( temporaryPath, "logRootPath" ),
            nginxSiteConfigRoot        : path.join( temporaryPath, "nginxSiteConfigRoot" ),
            fileStorageRoot            : path.join( temporaryPath, "fileStorageRoot" )
        };

        done();
    } );
} );

after( function( done ) {
    console.log( "Tearing down global test environment…" );

    onTeardown.forEach( function( toCall ) {
        try {
            toCall();

        } catch( error ) {
            console.log( "An error during teardown was ignored." );
            console.log( error.message );
        }
    } );

    done();
} );

[kfatehi]

I'm having a similar issue. My stack trace:

I20150523-20:51:19.146(-7)? Exception in callback of async function: Error: ENOTEMPTY, directory not empty '/var/folders/ry/v4yhrdtd2cs6bcbrbsl7f0hr0000gn/T/tmp-78691FSi1wtt5pxT5'
I20150523-20:51:19.146(-7)?     at Object.fs.rmdirSync (fs.js:624:18)
I20150523-20:51:19.146(-7)?     at _cleanupCallback (/Users/keyvan/Projects/fatehitech/packages/ftc/.npm/package/node_modules/tmp/lib/tmp.js:400:5)
I20150523-20:51:19.147(-7)?     at SpawnSession.failed (packages/ftc/deploy.js:39:1)
I20150523-20:51:19.147(-7)?     at runWithEnvironment (packages/meteor/dynamics_nodejs.js:108:1)
I20150523-20:51:19.147(-7)?     at Object.failed (packages/meteor/dynamics_nodejs.js:121:1)
I20150523-20:51:19.147(-7)?     at [object Object].SpawnSession.failed (packages/ftc/spawn-session.js:12:1)
I20150523-20:51:19.148(-7)?     at packages/ftc/deploy.js:18:1
I20150523-20:51:19.149(-7)?     at packages/ftc/spawn-session.js:26:1
I20150523-20:51:19.150(-7)?     at runWithEnvironment (packages/meteor/dynamics_nodejs.js:108:1)

I am calling cleanup({ unsafeCleanup: true }) due to my understanding that it removes everything recursively.

[chrisbartley]

Similar error with cleanup of temp files. Using Mac OS 10.9.5 and Node.js 0.10.36

In the example code below, the call to tempFileCleanupCallback() fails with error Error: EBADF, bad file descriptor. Also, it's not just the explicit cleanup that's failing. If I comment out the call to tempFileCleanupCallback(), the temp file still exists after process exit, even though I've called tmp.setGracefulCleanup();

Worked fine in v0.0.24. I was hoping to use 0.0.26 because my server is suffering from the memory leak reported in issue #48

var tmp = require('tmp');
var fs = require('fs');

tmp.setGracefulCleanup();

tmp.file({ prefix : 'tmp_test_', postfix : '.txt' },
      function(err, tempFilePath, tempFileDescriptor, tempFileCleanupCallback) {
         console.log("Temp file path: " + tempFilePath);
         if (err) {
            return console.log("Error creating the temp file: " + err);
         }

         fs.writeFile(tempFilePath,
                      "hello world",
                      function(err) {
                         if (err) {
                            return console.log("Error writing to the temp file: " + err);
                         }

                         fs.close(tempFileDescriptor,
                                  function(err) {
                                     if (err) {
                                        return console.log("Error closing the temp file: " + err);
                                     }

                                     // do something interesting with the temp file here...

                                     try {
                                        tempFileCleanupCallback();
                                     }
                                     catch (e) {
                                        console.log("Error trying to cleanup the temp file [" + tempFilePath + "]: " + e);
                                     }
                                  });
                      });
      });

[chrisbartley]

More detail...

I've found that cleanup works for this case:

var tmp = require('./lib/tmp');

tmp.setGracefulCleanup();

tmp.file({ prefix : 'tmp_test_', postfix : '.txt' },
      function(err, tempFilePath, tempFileDescriptor, tempFileCleanupCallback) {
         console.log("Temp file path: " + tempFilePath);
         if (err) {
            return console.log("Error creating the temp file: " + err);
         }
      });

And also this:

var tmp = require('./lib/tmp');

tmp.file({ prefix : 'tmp_test_', postfix : '.txt' },
      function(err, tempFilePath, tempFileDescriptor, tempFileCleanupCallback) {
         console.log("Temp file path: " + tempFilePath);
         if (err) {
            return console.log("Error creating the temp file: " + err);
         }

         console.log("Will attempt cleanup.");
         try {
            tempFileCleanupCallback();
         }
         catch (e) {
            console.log("Error trying to cleanup the temp file [" + tempFilePath + "]: " + e);
         }
      });

It's only if I actually attempt to do anything with the temp file, even simply trying to close it, that I get the Error: EBADF, bad file descriptor error. So, this fails with the Error: EBADF, bad file descriptor error:

var fs = require('fs');
var tmp = require('./lib/tmp');

tmp.file({ prefix : 'tmp_test_', postfix : '.txt' },
      function(err, tempFilePath, tempFileDescriptor, tempFileCleanupCallback) {
         console.log("Temp file path: " + tempFilePath);
         if (err) {
            return console.log("Error creating the temp file: " + err);
         }

         fs.close(tempFileDescriptor,
                  function(err) {
                     if (err) {
                        return console.log("Error closing the temp file: " + err);
                     }

                     console.log("Will attempt cleanup.");
                     try {
                        tempFileCleanupCallback();
                     }
                     catch (e) {
                        console.log("Error trying to cleanup the temp file [" + tempFilePath + "]: " + e);
                     }
                  });
      });

Similarly, this runs, with no error printed to the console, but the temp file doesn't actually get cleaned up on process exit:

var fs = require('fs');
var tmp = require('./lib/tmp');

tmp.setGracefulCleanup();

tmp.file({ prefix : 'tmp_test_', postfix : '.txt' },
      function(err, tempFilePath, tempFileDescriptor, tempFileCleanupCallback) {
         console.log("Temp file path: " + tempFilePath);
         if (err) {
            return console.log("Error creating the temp file: " + err);
         }

         fs.close(tempFileDescriptor,
                  function(err) {
                     if (err) {
                        return console.log("Error closing the temp file: " + err);
                     }
                  });
      });

My small brain is still trying to grok the test framework to understand why all tests pass. I can only guess I'm doing something dumb (very likely) or the tests aren't actually trying to do anything with the temp files before doing a cleanup.

[chrisbartley]

A-ha! The problem is that I was manually closing the temp file. Commit 3f70c3c added automatic closing of the temp file upon cleanup. I'm not sure that's super intuitive, but it is sort of mentioned in the README ("...the file will be closed and unlinked on process exit"). Perhaps make it clearer that manually calling the cleanup callback will also close the file before unlinking. Maybe _prepareTmpFileRemoveCallback() could simply swallow the exception thrown when it tries to close a file that was already closed (i.e. by a naive user like myself), something like this...

function _prepareTmpFileRemoveCallback(name, fd, opts) {
  var removeCallback = _prepareRemoveCallback(function _removeCallback(fdPath) {
    try {
      fs.closeSync(fdPath[0]);
    } catch (e) {
      console.log("Ignoring failure to close temp file [" + name + "]");
    }
    fs.unlinkSync(fdPath[1]);
  }, [fd, name]);

  if (!opts.keep) {
    _removeObjects.unshift(removeCallback);
  }

  return removeCallback;
}

But I think a bigger question is why the cleanup should be responsible for closing the file. I'd guess the most typical use case is that the user creates a temp file, writes to it, and then hands it off to some other process. With the current implementation, I have to leave the file open after writing before handing off to the other process. That feels odd. Is it not natural to assume you need to close a file after writing?

[silkentrance]

This directly relates to #58

[silkentrance]

@chrisbartley the automatic closing is a convenience, however, and your proposal is absolutely correct, tmp should handle thus arising errors more graciously. See also PR #47.

[chrisbartley]

While you're at it, please reopen issue 48 since the cleanup memory leak still exists. See #48 (comment)