Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_garbageCollector leaks if keep is false #48

Closed
bmeck opened this issue Mar 27, 2015 · 4 comments
Closed

_garbageCollector leaks if keep is false #48

bmeck opened this issue Mar 27, 2015 · 4 comments

Comments

@bmeck
Copy link
Contributor

bmeck commented Mar 27, 2015

_removeObjects never gets cleaned up if you call a cleanup function. it only ever gets appended to, this causes leaks for long lived processes that create many temporary file/dirs. when cleanup functions are called they should remove their temp file from _removedObjects.

the workaround right now is to always set keep:true and assume you are doing cleanup yourself

@bmeck
Copy link
Contributor Author

bmeck commented May 11, 2015

@raszi ^

@raszi raszi closed this as completed in 36a63fe May 12, 2015
raszi added a commit that referenced this issue May 12, 2015
remove object from _removeObjects if cleanup fn is called fixes #48
@chrisbartley
Copy link

This should be reopened. It still leaks when keep:false.

Here's an example which demonstrates the leak. First, add an else clause to _prepareRemoveCallback which prints out the size of the _removeObjects array, and add some logging to the if clause:

function _prepareRemoveCallback(removeFunction, arg) {
  var called = false;

  return function _cleanupCallback() {
    if (called) return;

    var index = _removeObjects.indexOf(removeFunction);
    if (index >= 0) {
      console.log("removing from _removeObjects");
      _removeObjects.splice(index, 1);
    } else {
      console.log("_removeObjects.length = [" + _removeObjects.length + "]");
    }

    called = true;
    removeFunction(arg);
  };
}

Then run the following:

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

var createTempFile = function() {
   tmp.file({ prefix : 'tmp_memory_leak_test_', postfix : '.txt', keep : false },
         function(err, tempFilePath, tempFileDescriptor, tempFileCleanupCallback) {
            if (err) {
               console.log("Error trying to create the temp file.  Will attempt cleanup.");
               tempFileCleanupCallback();
            }
            else {
               fs.writeFile(tempFilePath,
                            "Hello world, the current time is: " + Date.now() + "\n",
                            function(err) {
                               if (err) {
                                  console.log("Error trying to write the file.  Will attempt cleanup.");
                                  tempFileCleanupCallback();
                               }
                               else {
                                  tempFileCleanupCallback();
                                  setTimeout(createTempFile, 1);
                               }
                            });
            }
         });
};

createTempFile();

You'll see that removing from _removeObjects is never printed (because _removeObjects.indexOf(removeFunction) always returns -1) and that _removeObjects.length increases indefinitely.

@silkentrance
Copy link
Collaborator

@chrisbartley bummer, but, it is rather late right now, I will have a look at this tomorrow evening 😄

@silkentrance
Copy link
Collaborator

@chrisbartley having run some tests in the REPL with the extracted _prepareRemoveCallback function yields no errors, the _removeObjects array gets cleaned up properly. Will try again tomorrow evening with a proper test case.

raszi added a commit that referenced this issue Aug 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants