From 37b1af51df5a1ffcc5e516adc32ae422a6e15612 Mon Sep 17 00:00:00 2001
From: Christopher Hiller <boneskull@boneskull.com>
Date: Sun, 28 Jan 2018 13:33:21 -0800
Subject: [PATCH] fix memory leak when run in v8; closes #3119

- this means "multiple calls to done" will once again fail to produce a stack trace in the case that those multiple calls happen in separate tasks.
- however, if the `done()` is called with an error, Mocha will emit that error, but also a note about multiple calls
- some reformatting, evidently
- removed some pointless checks of Mocha's exit code from "multiple done" integration tests

Signed-off-by: Christopher Hiller <boneskull@boneskull.com>
---
 lib/runnable.js                               | 12 ++++--
 .../multiple-done-with-error.fixture.js       |  8 ++++
 test/integration/multiple-done.spec.js        | 41 +++++++++++++------
 3 files changed, 46 insertions(+), 15 deletions(-)
 create mode 100644 test/integration/fixtures/multiple-done-with-error.fixture.js

diff --git a/lib/runnable.js b/lib/runnable.js
index 694d4f59c2..e8ca41800f 100644
--- a/lib/runnable.js
+++ b/lib/runnable.js
@@ -53,7 +53,6 @@ function Runnable (title, fn) {
   this._slow = 75;
   this._enableTimeouts = true;
   this.timedOut = false;
-  this._trace = new Error('done() called multiple times');
   this._retries = -1;
   this._currentRetry = 0;
   this.pending = false;
@@ -278,7 +277,13 @@ Runnable.prototype.run = function (fn) {
       return;
     }
     emitted = true;
-    self.emit('error', err || new Error('done() called multiple times; stacktrace may be inaccurate'));
+    var msg = 'done() called multiple times';
+    if (err && err.message) {
+      err.message += " (and Mocha's " + msg + ')';
+      self.emit('error', err);
+    } else {
+      self.emit('error', new Error(msg));
+    }
   }
 
   // finished
@@ -287,8 +292,9 @@ Runnable.prototype.run = function (fn) {
     if (self.timedOut) {
       return;
     }
+
     if (finished) {
-      return multiple(err || self._trace);
+      return multiple(err);
     }
 
     self.clearTimeout();
diff --git a/test/integration/fixtures/multiple-done-with-error.fixture.js b/test/integration/fixtures/multiple-done-with-error.fixture.js
new file mode 100644
index 0000000000..4d4c55c7d2
--- /dev/null
+++ b/test/integration/fixtures/multiple-done-with-error.fixture.js
@@ -0,0 +1,8 @@
+'use strict';
+
+it('should fail in a test-case', function (done) {
+  process.nextTick(function () {
+    done();
+    done(new Error('second error'));
+  });
+});
diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js
index 76a6483115..3ac216e457 100644
--- a/test/integration/multiple-done.spec.js
+++ b/test/integration/multiple-done.spec.js
@@ -15,15 +15,35 @@ describe('multiple calls to done()', function () {
     });
 
     it('results in failures', function () {
-      assert.equal(res.stats.pending, 0);
-      assert.equal(res.stats.passes, 1);
-      assert.equal(res.stats.failures, 1);
-      assert.equal(res.code, 1);
+      assert.equal(res.stats.pending, 0, 'wrong "pending" count');
+      assert.equal(res.stats.passes, 1, 'wrong "passes" count');
+      assert.equal(res.stats.failures, 1, 'wrong "failures" count');
     });
 
     it('throws a descriptive error', function () {
-      assert.equal(res.failures[0].err.message,
-        'done() called multiple times');
+      assert.equal(res.failures[0].err.message, 'done() called multiple times');
+    });
+  });
+
+  describe('with error passed on second call', function () {
+    before(function (done) {
+      run('multiple-done-with-error.fixture.js', args, function (err, result) {
+        res = result;
+        done(err);
+      });
+    });
+
+    it('results in failures', function () {
+      assert.equal(res.stats.pending, 0, 'wrong "pending" count');
+      assert.equal(res.stats.passes, 1, 'wrong "passes" count');
+      assert.equal(res.stats.failures, 1, 'wrong "failures" count');
+    });
+
+    it('should throw a descriptive error', function () {
+      assert.equal(
+        res.failures[0].err.message,
+        "second error (and Mocha's done() called multiple times)"
+      );
     });
   });
 
@@ -44,8 +64,7 @@ describe('multiple calls to done()', function () {
 
     it('correctly attributes the error', function () {
       assert.equal(res.failures[0].fullTitle, 'suite test1');
-      assert.equal(res.failures[0].err.message,
-        'done() called multiple times');
+      assert.equal(res.failures[0].err.message, 'done() called multiple times');
     });
   });
 
@@ -66,8 +85,7 @@ describe('multiple calls to done()', function () {
 
     it('correctly attributes the error', function () {
       assert.equal(res.failures[0].fullTitle, 'suite "before all" hook');
-      assert.equal(res.failures[0].err.message,
-        'done() called multiple times');
+      assert.equal(res.failures[0].err.message, 'done() called multiple times');
     });
   });
 
@@ -90,8 +108,7 @@ describe('multiple calls to done()', function () {
       assert.equal(res.failures.length, 2);
       res.failures.forEach(function (failure) {
         assert.equal(failure.fullTitle, 'suite "before each" hook');
-        assert.equal(failure.err.message,
-          'done() called multiple times');
+        assert.equal(failure.err.message, 'done() called multiple times');
       });
     });
   });