stop mining on close (#185)

* stop mining on close

* when closing provider do not pass callback to stopMining

* add test

* fix that new mining test

* fix race condition in mineOnInterval

* update tests

lib/provider.js

@@ -132,6 +132,7 @@ Provider.prototype.send = function(payload, callback) {
 
 Provider.prototype.close = function(callback) {
   // This is a little gross reaching, but...
+  this.manager.state.stopMining();
   this.manager.state.blockchain.close(callback);
   this.engine.stop();
 };

lib/statemanager.js

@@ -166,25 +166,40 @@ StateManager.prototype.initialize = function(callback) {
   });
 };
 
+StateManager.prototype._minerCancellationToken = null;
 StateManager.prototype.mineOnInterval = function() {
-  var self = this;
+  // cancel the a previous miner's timeout
+  clearTimeout(this.mining_interval_timeout);
 
-  // For good measure.
-  clearTimeout(self.mining_interval_timeout);
-  self.mining_interval_timeout = null;
+  // make sure a pending eth_mine doesn't come back and execute mineOnInterval
+  // again...
+  if (this._minerCancellationToken !== null) {
+    this._minerCancellationToken.cancelled = true;
+  }
 
-  self.mining_interval_timeout = setTimeout(function() {
-    self._provider.send(
-      {
-        method: "evm_mine"
-      },
-      self.mineOnInterval.bind(self)
-    );
-  }, this.blockTime * 1000);
+  // if mining was stopped `mineOnInterval` shouldn't start mining again
+  if (!this.is_mining) {
+    this.logger.log("Warning: mineOnInterval called when miner was stopped");
+    return;
+  }
+
+  const cancellationToken = { cancelled: false };
+  this._minerCancellationToken = cancellationToken;
+
+  const timeout = (this.mining_interval_timeout = setTimeout(
+    this._provider.send.bind(this._provider),
+    this.blockTime * 1000,
+    { method: "evm_mine" },
+    () => {
+      if (!cancellationToken.cancelled) {
+        this.mineOnInterval.bind(this)();
+      }
+    }
+  ));
 
   // Ensure this won't keep a node process open.
-  if (this.mining_interval_timeout && this.mining_interval_timeout.unref) {
-    this.mining_interval_timeout.unref();
+  if (typeof timeout.unref === "function") {
+    timeout.unref();
   }
 };
 
@@ -887,6 +902,12 @@ StateManager.prototype.revert = function(snapshotId, callback) {
 };
 
 StateManager.prototype.startMining = function(callback) {
+  if (this.is_mining) {
+    callback();
+    this.logger.log("Warning: startMining called when miner was already started");
+    return;
+  }
+
   this.is_mining = true;
 
   if (this.is_mining_on_interval) {
@@ -898,10 +919,18 @@ StateManager.prototype.startMining = function(callback) {
 };
 
 StateManager.prototype.stopMining = function(callback) {
-  this.is_mining = false;
-  clearTimeout(this.mining_interval_timeout);
-  this.mining_interval_timeout = null;
-  callback();
+  if (this.is_mining) {
+    if (this._minerCancellationToken) {
+      this._minerCancellationToken.cancelled = true;
+      this._minerCancellationToken = null;
+    }
+    this.is_mining = false;
+    clearTimeout(this.mining_interval_timeout);
+    this.mining_interval_timeout = null;
+  } else {
+    this.logger.log("Warning: stopMining called when miner was already stopped");
+  }
+  callback && callback();
 };
 
 StateManager.prototype.isUnlocked = function(address) {

test/mining.js

@@ -355,4 +355,52 @@ describe("Mining", function() {
     isMining = await checkMining();
     assert(isMining);
   });
+
+  describe("stopping", () => {
+    function setUp(close, done) {
+      const blockTime = 0.1;
+      const provider = Ganache.provider({ blockTime });
+      let closed = false;
+      let closing = false;
+      let timer;
+
+      // duck punch provider.send so we can detect when it is called
+      const send = provider.send;
+      provider.send = function(payload) {
+        if (payload.method === "evm_mine") {
+          if (closed) {
+            clearTimeout(timer);
+            assert.fail("evm_mine after provider closed");
+          } else if (!closing) {
+            closing = true;
+            close(provider, () => {
+              closed = true;
+
+              // give the miner a chance to mine a block before calling done:
+              timer = setTimeout(done, blockTime * 2 * 1000);
+            });
+          }
+        }
+        send.apply(provider, arguments);
+      };
+    }
+
+    it("should stop mining when the provider is stopped during an evm_mine (same REPL)", (done) => {
+      setUp(function(provider, callback) {
+        provider.close(callback);
+      }, done);
+    });
+
+    it("should stop mining when the provider is stopped during evm_mine (next tick)", (done) => {
+      setUp(function(provider, callback) {
+        process.nextTick(() => provider.close(callback));
+      }, done);
+    });
+
+    it("should stop mining when the provider is stopped during evm_mine (setImmediate)", (done) => {
+      setUp(function(provider, callback) {
+        setImmediate(() => provider.close(callback));
+      }, done);
+    });
+  });
 });