From a16fdec23b455995fa5ab53ab3a57cea2cd8742e Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Mon, 27 Aug 2018 10:27:06 +0200 Subject: [PATCH] fix(lnd): ensure graceful shutdown Lnd doesn't currently do a graceful shutdown when a `SIGTERM` is issued. To ensure a graceful shutdown, call `stopDaemon` on the gRPC interface prior to terminating lnd. If a graceful shutdown is unsuccessful then force kill the daemon. --- app/lib/lnd/lightning.js | 69 +++++++++++++-- app/lib/lnd/neutrino.js | 8 +- app/lib/zap/controller.js | 148 ++++++++++++++++++++------------- test/unit/lnd/neutrino.spec.js | 4 +- 4 files changed, 159 insertions(+), 70 deletions(-) diff --git a/app/lib/lnd/lightning.js b/app/lib/lnd/lightning.js index 81e466e3..170a3816 100644 --- a/app/lib/lnd/lightning.js +++ b/app/lib/lnd/lightning.js @@ -3,6 +3,7 @@ import grpc from 'grpc' import { loadSync } from '@grpc/proto-loader' import { BrowserWindow } from 'electron' +import StateMachine from 'javascript-state-machine' import LndConfig from './config' import { getDeadline, validateHost, createSslCreds, createMacaroonCreds } from './util' import methods from './methods' @@ -25,24 +26,43 @@ type LightningSubscriptionsType = { class Lightning { mainWindow: BrowserWindow lnd: any + lndConfig: LndConfig subscriptions: LightningSubscriptionsType + _fsm: StateMachine - constructor() { + // Transitions provided by the state machine. + connect: any + disconnect: any + terminate: any + is: any + can: any + state: string + + constructor(lndConfig: LndConfig) { this.mainWindow = null this.lnd = null + this.lndConfig = lndConfig this.subscriptions = { channelGraph: null, invoices: null, transactions: null } + + // Initialize the state machine. + this._fsm() } + // ------------------------------------ + // FSM Callbacks + // ------------------------------------ + /** * Connect to the gRPC interface and verify it is functional. * @return {Promise} */ - async connect(lndConfig: LndConfig) { - const { rpcProtoPath, host, cert, macaroon } = lndConfig + async onBeforeConnect() { + mainLog.info('Connecting to Lightning gRPC service') + const { rpcProtoPath, host, cert, macaroon } = this.lndConfig // Verify that the host is valid before creating a gRPC client that is connected to it. return await validateHost(host).then(async () => { @@ -69,16 +89,15 @@ class Lightning { const credentials = grpc.credentials.combineChannelCredentials(sslCreds, macaroonCreds) // Create a new gRPC client instance. - const lnd = new rpc.lnrpc.Lightning(host, credentials) + this.lnd = new rpc.lnrpc.Lightning(host, credentials) - // Call the getInfo method to ensure that we can make successful calls to the gRPC interface. + // Wait for the gRPC connection to be established. return new Promise((resolve, reject) => { - lnd.getInfo({}, { deadline: getDeadline(2) }, err => { + grpc.waitForClientReady(this.lnd, getDeadline(2), err => { if (err) { return reject(err) } - this.lnd = lnd - return resolve(lnd) + return resolve() }) }) }) @@ -87,13 +106,34 @@ class Lightning { /** * Discomnnect the gRPC service. */ - disconnect() { + onBeforeDisconnect() { + mainLog.info('Disconnecting from Lightning gRPC service') this.unsubscribe() if (this.lnd) { this.lnd.close() } } + /** + * Gracefully shutdown the gRPC service. + */ + async onBeforeTerminate() { + mainLog.info('Shutting down Lightning daemon') + this.unsubscribe() + return new Promise((resolve, reject) => { + this.lnd.stopDaemon({}, (err, data) => { + if (err) { + return reject(err) + } + resolve(data) + }) + }) + } + + // ------------------------------------ + // Helpers + // ------------------------------------ + /** * Hook up lnd restful methods. */ @@ -105,6 +145,7 @@ class Lightning { * Subscribe to all bi-directional streams. */ subscribe(mainWindow: BrowserWindow) { + mainLog.info('Subscribing to Lightning gRPC streams') this.mainWindow = mainWindow this.subscriptions.channelGraph = subscribeToChannelGraph.call(this) @@ -116,6 +157,7 @@ class Lightning { * Unsubscribe from all bi-directional streams. */ unsubscribe() { + mainLog.info('Unsubscribing from Lightning gRPC streams') this.mainWindow = null Object.keys(this.subscriptions).forEach(subscription => { if (this.subscriptions[subscription]) { @@ -126,4 +168,13 @@ class Lightning { } } +StateMachine.factory(Lightning, { + init: 'ready', + transitions: [ + { name: 'connect', from: 'ready', to: 'connected' }, + { name: 'disconnect', from: 'connected', to: 'ready' }, + { name: 'terminate', from: 'connected', to: 'ready' } + ] +}) + export default Lightning diff --git a/app/lib/lnd/neutrino.js b/app/lib/lnd/neutrino.js index 65299999..23914b07 100644 --- a/app/lib/lnd/neutrino.js +++ b/app/lib/lnd/neutrino.js @@ -141,7 +141,10 @@ class Neutrino extends EventEmitter { if (this.is(CHAIN_SYNC_PENDING) || this.is(CHAIN_SYNC_IN_PROGRESS)) { // If we cant get a connectionn to the backend. - if (line.includes('Waiting for chain backend to finish sync')) { + if ( + line.includes('Waiting for chain backend to finish sync') || + line.includes('Waiting for block headers to sync, then will start cfheaders sync') + ) { this.setState(CHAIN_SYNC_WAITING) } // If we are still waiting for the back end to finish synncing. @@ -223,8 +226,9 @@ class Neutrino extends EventEmitter { /** * Stop the Lnd process. */ - stop() { + kill() { if (this.process) { + mainLog.info('Killing Neutrino process...') this.process.kill() this.process = null } diff --git a/app/lib/zap/controller.js b/app/lib/zap/controller.js index 25e12bc7..794f867a 100644 --- a/app/lib/zap/controller.js +++ b/app/lib/zap/controller.js @@ -51,10 +51,9 @@ const grpcSslCipherSuites = connectionType => */ class ZapController { mainWindow: BrowserWindow - neutrino: any - lightning: any + neutrino: Neutrino + lightning: Lightning splashScreenTime: number - lightningGrpcConnected: boolean lndConfig: LndConfig _fsm: StateMachine @@ -73,18 +72,9 @@ class ZapController { // Variable to hold the main window instance. this.mainWindow = mainWindow - // Keep a reference any neutrino process started by us. - this.neutrino = undefined - - // Keep a reference to the lightning gRPC instance. - this.lightning = undefined - // Time for the splash screen to remain visible. this.splashScreenTime = 500 - // Boolean indicating wether the lightning grpc is connected ot not. - this.lightningGrpcConnected = false - // Initialize the state machine. this._fsm() @@ -127,7 +117,7 @@ class ZapController { // FSM Callbacks // ------------------------------------ - onOnboarding() { + async onOnboarding(lifecycle: any) { mainLog.debug('[FSM] onOnboarding...') // Remove any existing IPC listeners so that we can start fresh. @@ -136,12 +126,14 @@ class ZapController { // Register IPC listeners so that we can react to instructions coming from the app. this._registerIpcListeners() - // Ensure wallet is disconnected. - this.disconnectLightningWallet() + // Disconnect any pre-existing lightning wallet connection. + if (lifecycle.from === 'connected' && this.lightning && this.lightning.can('disconnect')) { + this.lightning.disconnect() + } - // If Neutrino is running, kill it. - if (this.neutrino) { - this.neutrino.stop() + // If we are comming from a running state, stop the Neutrino process. + else if (lifecycle.from === 'running') { + await this.shutdownNeutrino() } // Give the grpc connections a chance to be properly closed out. @@ -209,15 +201,16 @@ class ZapController { }) } - onTerminated() { + async onTerminated(lifecycle: any) { mainLog.debug('[FSM] onTerminated...') - // Unsubscribe the gRPC streams before thhe window closes. This ensures that we can properly reestablish a fresh - // connection when a new window is opened. - this.disconnectLightningWallet() - // If Neutrino is running, kill it. - if (this.neutrino) { - this.neutrino.stop() + // Disconnect from any existing lightning wallet connection. + if (lifecycle.from === 'connected' && this.lightning && this.lightning.can('disconnect')) { + this.lightning.disconnect() + } + // If we are comming from a running state, stop the Neutrino process. + else if (lifecycle.from === 'running') { + await this.shutdownNeutrino() } } @@ -251,7 +244,7 @@ class ZapController { * Start the wallet unlocker. */ startWalletUnlocker() { - mainLog.info('Starting wallet unlocker...') + mainLog.info('Establishing connection to Wallet Unlocker gRPC interface...') try { const walletUnlockerMethods = initWalletUnlocker(this.lndConfig) @@ -275,42 +268,23 @@ class ZapController { * Create and subscribe to the Lightning service. */ async startLightningWallet() { - if (this.lightningGrpcConnected) { - return - } - mainLog.info('Starting lightning wallet...') - this.lightning = new Lightning() + mainLog.info('Establishing connection to Lightning gRPC interface...') + this.lightning = new Lightning(this.lndConfig) // Connect to the Lightning interface. - await this.lightning.connect(this.lndConfig) - - // Subscribe the main window to receive streams. - this.lightning.subscribe(this.mainWindow) - - // Listen for all gRPC restful methods and pass to gRPC. - ipcMain.on('lnd', (event, { msg, data }) => this.lightning.lndMethods(event, msg, data)) + try { + await this.lightning.connect() - // Let the renderer know that we are connected. - this.sendMessage('lightningGrpcActive') + this.lightning.subscribe(this.mainWindow) - // Update our internal state. - this.lightningGrpcConnected = true - } + // Listen for all gRPC restful methods and pass to gRPC. + ipcMain.on('lnd', (event, { msg, data }) => this.lightning.lndMethods(event, msg, data)) - /** - * Unsubscribe from the Lightning service. - */ - disconnectLightningWallet() { - if (!this.lightningGrpcConnected) { - return + // Let the renderer know that we are connected. + this.sendMessage('lightningGrpcActive') + } catch (err) { + mainLog.warn('Unable to connect to Lighitnng gRPC interface: %o', err) } - mainLog.info('Disconnecting lightning Wallet...') - - // Disconnect streams. - this.lightning.disconnect() - - // Update our internal state. - this.lightningGrpcConnected = false } /** @@ -337,7 +311,7 @@ class ZapController { if (this.is('running') || this.is('connected')) { dialog.showMessageBox({ type: 'error', - message: `Lnd has unexpectadly quit: ${lastError}` + message: `Lnd has unexpectedly quit: ${lastError}` }) this.terminate() } @@ -383,6 +357,66 @@ class ZapController { this.neutrino.start() } + /** + * Gracefully shutdown LND. + */ + async shutdownNeutrino() { + // We only want to shut down LND if we are running it locally. + if (this.lndConfig.type !== 'local') { + return Promise.resolve() + } + + // Attempt a graceful shutdown if we can. + if (this.lightning && this.lightning.can('terminate')) { + mainLog.info('Shutting down Neutrino...') + + return new Promise(resolve => { + // HACK: Sometimes there are errors during the shutdown process that prevent the daeming from shutting down at + // all. If we haven't received notification of the process closing within 10 seconds, kill it. + // See https://github.com/lightningnetwork/lnd/pull/1781 + // See https://github.com/lightningnetwork/lnd/pull/1783 + const shutdownTimeout = setTimeout(() => { + this.neutrino.removeListener('close', closeHandler) + if (this.neutrino) { + mainLog.warn('Graceful shutdown failed to complete within 30 seconds.') + this.neutrino.kill() + resolve() + } + }, 1000 * 10) + + // HACK: The Lightning.stopDaemon` call returns before lnd has actually fully completed the shutdown process + // so we add a listener on the close event so that we can wrap things up once the process has been fully closed + // out. + const closeHandler = function() { + mainLog.info('Neutrino shutdown complete.') + clearTimeout(shutdownTimeout) + resolve() + } + this.neutrino.once('close', closeHandler) + + this.lightning + .terminate() + .then(() => mainLog.info('Neutrino Daemon shutdown complete')) + .catch(err => { + mainLog.error('Unable to gracefully shutdown LND: %o', err) + // Kill the process ourselves here to ensure that we don't leave hanging processes. + if (this.neutrino) { + this.neutrino.kill() + resolve() + } + }) + }) + } + + // The Lightning service is only active after the wallet has been unlocked and a gRPC connection has been + // established. In this case, kill the Neutrino process to ensure that we don't leave hanging process. + // FIXME: This currencly doesn't do a graceful shutdown as LND does not properly handle SIGTERM. + // See https://github.com/lightningnetwork/lnd/issues/1028 + else if (this.neutrino) { + this.neutrino.kill() + } + } + finishOnboarding(options: onboardingOptions) { mainLog.info('Finishing onboarding') // Save the lnd config options that we got from the renderer. diff --git a/test/unit/lnd/neutrino.spec.js b/test/unit/lnd/neutrino.spec.js index 71827a27..2b7dcaba 100644 --- a/test/unit/lnd/neutrino.spec.js +++ b/test/unit/lnd/neutrino.spec.js @@ -177,14 +177,14 @@ describe('Neutrino', function() { }) }) - describe('.stop', () => { + describe('.kill', () => { describe('called when neutrino is already running', () => { beforeEach(() => { this.neutrino = new Neutrino() this.neutrino.process = { kill: jest.fn() } - this.neutrino.stop() + this.neutrino.kill() }) it('should kill the neutrino process', () => { expect(this.neutrino.process).toBeNull()