From dc6d0e681c92c6d30789be73d77e77471bf3006d Mon Sep 17 00:00:00 2001 From: Chris Kleeschulte Date: Tue, 15 Sep 2015 14:18:18 -0400 Subject: [PATCH 1/4] Crash on reindex - Added the concept of loadServices on the node so that the node can conditionally call stop on loadingServices - This serves the case where services might be loading versus fully loaded (which is not always the cases for heavy services like bitcoind) --- lib/node.js | 6 ++++-- test/node.unit.js | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/node.js b/lib/node.js index 3061610c..b58356a1 100644 --- a/lib/node.js +++ b/lib/node.js @@ -22,6 +22,7 @@ function Node(config) { this.network = null; this.services = {}; this._unloadedServices = []; + this._loadingServices = {}; // TODO type check the arguments of config.services if (config.services) { @@ -140,6 +141,7 @@ Node.prototype._startService = function(serviceInfo, callback) { config.node = this; config.name = serviceInfo.name; var service = new serviceInfo.module(config); + self._loadingServices[service.name] = service; service.start(function(err) { if (err) { @@ -206,9 +208,9 @@ Node.prototype.stop = function(callback) { async.eachSeries( services, function(service, next) { - if (self.services[service.name]) { + if (self._loadingServices[service.name]) { log.info('Stopping ' + service.name); - self.services[service.name].stop(next); + self._loadingServices[service.name].stop(next); } else { log.info('Stopping ' + service.name + ' (not started)'); setImmediate(next); diff --git a/test/node.unit.js b/test/node.unit.js index 6593f7b5..d1589152 100644 --- a/test/node.unit.js +++ b/test/node.unit.js @@ -333,8 +333,10 @@ describe('Bitcore Node', function() { ['getData', this, this.getData, 1] ]; }; + var testService = new TestService({node: node}); + node._loadingServices = {'test1': testService}; node.services = { - 'test1': new TestService({node: node}) + 'test1': testService }; node.test2 = {}; node.test2.stop = sinon.stub().callsArg(0); From 4ee11ed73b7a1b3edc51cfdb8d05b484a7f603cf Mon Sep 17 00:00:00 2001 From: Chris Kleeschulte Date: Tue, 15 Sep 2015 16:38:41 -0400 Subject: [PATCH 2/4] Crash on reindex - Introduced the concept of a Cancellation error so that services can choose to watch for a cancellation flag. - Services can then send this error back and it will be forwarded to the node. - The node will then know to call shutdown appropriately. --- lib/errors.js | 5 ++++- lib/node.js | 23 +++++++++++++++-------- lib/scaffold/start.js | 6 +++++- lib/services/bitcoind.js | 5 +++++ test/node.unit.js | 5 ++--- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index 0a1b6a42..420e4fc1 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -15,11 +15,14 @@ Consensus.BlockExists = createError('BlockExists', Consensus); var Transaction = createError('Transaction', BitcoreNodeError); Transaction.NotFound = createError('NotFound', Transaction); +var Cancellation = createError('Cancellation', BitcoreNodeError); + module.exports = { Error: BitcoreNodeError, NoOutputs: NoOutputs, NoOutput: NoOutput, Wallet: Wallet, Consensus: Consensus, - Transaction: Transaction + Transaction: Transaction, + Cancellation: Cancellation }; diff --git a/lib/node.js b/lib/node.js index b58356a1..6a30ea41 100644 --- a/lib/node.js +++ b/lib/node.js @@ -22,7 +22,7 @@ function Node(config) { this.network = null; this.services = {}; this._unloadedServices = []; - this._loadingServices = {}; + this.started = false; // TODO type check the arguments of config.services if (config.services) { @@ -141,16 +141,15 @@ Node.prototype._startService = function(serviceInfo, callback) { config.node = this; config.name = serviceInfo.name; var service = new serviceInfo.module(config); - self._loadingServices[service.name] = service; + + // include in loaded services + self.services[serviceInfo.name] = service; service.start(function(err) { if (err) { return callback(err); } - // include in loaded services - self.services[serviceInfo.name] = service; - // add API methods var methodData = service.getAPIMethods(); var methodNameConflicts = []; @@ -188,16 +187,24 @@ Node.prototype.start = function(callback) { self._startService(service, next); }, function(err) { - if (err) { + if (err instanceof errors.Cancellation) { + self.cancellation = true; + return callback(err); + } else if (err) { return callback(err); } self.emit('ready'); + self.started = true; callback(); } ); }; Node.prototype.stop = function(callback) { + if (!this.started && !this.cancellation) { + this.pendingStop = true; + return; + } log.info('Beginning shutdown'); var self = this; var services = this.getServiceOrder().reverse(); @@ -208,9 +215,9 @@ Node.prototype.stop = function(callback) { async.eachSeries( services, function(service, next) { - if (self._loadingServices[service.name]) { + if (self.services[service.name]) { log.info('Stopping ' + service.name); - self._loadingServices[service.name].stop(next); + self.services[service.name].stop(next); } else { log.info('Stopping ' + service.name + ' (not started)'); setImmediate(next); diff --git a/lib/scaffold/start.js b/lib/scaffold/start.js index 0d24228e..1f193518 100644 --- a/lib/scaffold/start.js +++ b/lib/scaffold/start.js @@ -7,6 +7,7 @@ var bitcore = require('bitcore'); var _ = bitcore.deps._; var $ = bitcore.util.preconditions; var log = index.log; +var errors = index.errors; var child_process = require('child_process'); var fs = require('fs'); var shuttingDown = false; @@ -215,7 +216,10 @@ function start(options) { }); node.start(function(err) { - if(err) { + if (err instanceof errors.Cancellation) { + log.warn('Cancelled start up of all services'); + start.cleanShutdown(process, node); + } else if(err) { log.error('Failed to start services'); if (err.stack) { log.error(err.stack); diff --git a/lib/services/bitcoind.js b/lib/services/bitcoind.js index 2eca3cb4..8f5b7f10 100644 --- a/lib/services/bitcoind.js +++ b/lib/services/bitcoind.js @@ -9,6 +9,7 @@ var $ = bitcore.util.preconditions; var _ = bitcore.deps._; var index = require('../'); var log = index.log; +var errors = index.errors; var Service = require('../service'); /** @@ -158,6 +159,10 @@ Bitcoin.prototype.start = function(callback) { } if (self._reindex) { var interval = setInterval(function() { + if (self.node.pendingStop) { + clearInterval(interval); + return callback(new errors.Cancellation(), false); + } var percentSynced = bindings.syncPercentage(); log.info("Bitcoin Core Daemon Reindex Percentage: " + percentSynced); if (percentSynced >= 100) { diff --git a/test/node.unit.js b/test/node.unit.js index d1589152..4216b8cc 100644 --- a/test/node.unit.js +++ b/test/node.unit.js @@ -324,6 +324,7 @@ describe('Bitcore Node', function() { describe('#stop', function() { it('will call stop for each service', function(done) { var node = new Node(baseConfig); + node.started = true; function TestService() {} util.inherits(TestService, BaseService); TestService.prototype.stop = sinon.stub().callsArg(0); @@ -333,10 +334,8 @@ describe('Bitcore Node', function() { ['getData', this, this.getData, 1] ]; }; - var testService = new TestService({node: node}); - node._loadingServices = {'test1': testService}; node.services = { - 'test1': testService + 'test1': new TestService({node: node}) }; node.test2 = {}; node.test2.stop = sinon.stub().callsArg(0); From c9d4dc276f0d2be9a447d489787b462642a41ae0 Mon Sep 17 00:00:00 2001 From: Chris Kleeschulte Date: Wed, 16 Sep 2015 10:22:24 -0400 Subject: [PATCH 3/4] Crash on reindex - Removed unneeded cancellation error and handlers for it. --- lib/errors.js | 5 +---- lib/node.js | 11 +---------- lib/scaffold/start.js | 5 +---- lib/services/bitcoind.js | 4 ---- test/node.unit.js | 1 - 5 files changed, 3 insertions(+), 23 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index 420e4fc1..0a1b6a42 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -15,14 +15,11 @@ Consensus.BlockExists = createError('BlockExists', Consensus); var Transaction = createError('Transaction', BitcoreNodeError); Transaction.NotFound = createError('NotFound', Transaction); -var Cancellation = createError('Cancellation', BitcoreNodeError); - module.exports = { Error: BitcoreNodeError, NoOutputs: NoOutputs, NoOutput: NoOutput, Wallet: Wallet, Consensus: Consensus, - Transaction: Transaction, - Cancellation: Cancellation + Transaction: Transaction }; diff --git a/lib/node.js b/lib/node.js index 6a30ea41..6309da6c 100644 --- a/lib/node.js +++ b/lib/node.js @@ -22,7 +22,6 @@ function Node(config) { this.network = null; this.services = {}; this._unloadedServices = []; - this.started = false; // TODO type check the arguments of config.services if (config.services) { @@ -187,24 +186,16 @@ Node.prototype.start = function(callback) { self._startService(service, next); }, function(err) { - if (err instanceof errors.Cancellation) { - self.cancellation = true; - return callback(err); - } else if (err) { + if (err) { return callback(err); } self.emit('ready'); - self.started = true; callback(); } ); }; Node.prototype.stop = function(callback) { - if (!this.started && !this.cancellation) { - this.pendingStop = true; - return; - } log.info('Beginning shutdown'); var self = this; var services = this.getServiceOrder().reverse(); diff --git a/lib/scaffold/start.js b/lib/scaffold/start.js index 1f193518..8c5f1629 100644 --- a/lib/scaffold/start.js +++ b/lib/scaffold/start.js @@ -216,10 +216,7 @@ function start(options) { }); node.start(function(err) { - if (err instanceof errors.Cancellation) { - log.warn('Cancelled start up of all services'); - start.cleanShutdown(process, node); - } else if(err) { + if(err) { log.error('Failed to start services'); if (err.stack) { log.error(err.stack); diff --git a/lib/services/bitcoind.js b/lib/services/bitcoind.js index 8f5b7f10..bda56272 100644 --- a/lib/services/bitcoind.js +++ b/lib/services/bitcoind.js @@ -159,10 +159,6 @@ Bitcoin.prototype.start = function(callback) { } if (self._reindex) { var interval = setInterval(function() { - if (self.node.pendingStop) { - clearInterval(interval); - return callback(new errors.Cancellation(), false); - } var percentSynced = bindings.syncPercentage(); log.info("Bitcoin Core Daemon Reindex Percentage: " + percentSynced); if (percentSynced >= 100) { diff --git a/test/node.unit.js b/test/node.unit.js index 4216b8cc..6593f7b5 100644 --- a/test/node.unit.js +++ b/test/node.unit.js @@ -324,7 +324,6 @@ describe('Bitcore Node', function() { describe('#stop', function() { it('will call stop for each service', function(done) { var node = new Node(baseConfig); - node.started = true; function TestService() {} util.inherits(TestService, BaseService); TestService.prototype.stop = sinon.stub().callsArg(0); From a105c0a35ecb08fca005a4abbd7ced6dc3260bc2 Mon Sep 17 00:00:00 2001 From: Chris Kleeschulte Date: Wed, 16 Sep 2015 10:38:08 -0400 Subject: [PATCH 4/4] Removed errors memoization. --- lib/scaffold/start.js | 1 - lib/services/bitcoind.js | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/scaffold/start.js b/lib/scaffold/start.js index 8c5f1629..0d24228e 100644 --- a/lib/scaffold/start.js +++ b/lib/scaffold/start.js @@ -7,7 +7,6 @@ var bitcore = require('bitcore'); var _ = bitcore.deps._; var $ = bitcore.util.preconditions; var log = index.log; -var errors = index.errors; var child_process = require('child_process'); var fs = require('fs'); var shuttingDown = false; diff --git a/lib/services/bitcoind.js b/lib/services/bitcoind.js index bda56272..2eca3cb4 100644 --- a/lib/services/bitcoind.js +++ b/lib/services/bitcoind.js @@ -9,7 +9,6 @@ var $ = bitcore.util.preconditions; var _ = bitcore.deps._; var index = require('../'); var log = index.log; -var errors = index.errors; var Service = require('../service'); /**