From 6300256a3e4d04795a14151b6183d8dadfbc3cb4 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Mon, 5 Nov 2018 11:06:12 -0800 Subject: [PATCH] Gracefully shut down database operations before app exit --- js/background.js | 4 +++ js/models/messages.js | 8 ++++- js/modules/data.js | 62 ++++++++++++++++++++++++++++++++++-- main.js | 74 +++++++++++++++++++++++++++++++++++++++---- preload.js | 24 +++++++++++++- 5 files changed, 161 insertions(+), 11 deletions(-) diff --git a/js/background.js b/js/background.js index 9a23591ea..180ee45a3 100644 --- a/js/background.js +++ b/js/background.js @@ -272,6 +272,10 @@ const clearDataView = new window.Whisper.ClearDataView().render(); $('body').append(clearDataView.el); }, + + shutdown: async () => { + await window.Signal.Data.shutdown(); + }, }; const currentVersion = window.getVersion(); diff --git a/js/models/messages.js b/js/models/messages.js index 92a570e3a..dd3d081e5 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -844,7 +844,13 @@ return this.OUR_NUMBER; }, getContact() { - return ConversationController.getOrCreate(this.getSource(), 'private'); + const source = this.getSource(); + + if (!source) { + return null; + } + + return ConversationController.getOrCreate(source, 'private'); }, isOutgoing() { return this.get('type') === 'outgoing'; diff --git a/js/modules/data.js b/js/modules/data.js index ae886c206..a0a81c920 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -32,6 +32,9 @@ const CLEANUP_ORPHANED_ATTACHMENTS_KEY = 'cleanup-orphaned-attachments'; const _jobs = Object.create(null); const _DEBUG = false; let _jobCounter = 0; +let _shuttingDown = false; +let _shutdownCallback = null; +let _shutdownPromise = null; const channels = {}; @@ -39,6 +42,7 @@ module.exports = { _jobs, _cleanData, + shutdown, close, removeDB, removeIndexedDBFiles, @@ -172,7 +176,45 @@ function _cleanData(data) { return data; } +async function _shutdown() { + if (_shutdownPromise) { + return _shutdownPromise; + } + + _shuttingDown = true; + + const jobKeys = Object.keys(_jobs); + window.log.info( + `data.shutdown: starting process. ${jobKeys.length} jobs outstanding` + ); + + // No outstanding jobs, return immediately + if (jobKeys.length === 0) { + return null; + } + + // Outstanding jobs; we need to wait until the last one is done + _shutdownPromise = new Promise((resolve, reject) => { + _shutdownCallback = error => { + window.log.info('data.shutdown: process complete'); + if (error) { + return reject(error); + } + + return resolve(); + }; + }); + + return _shutdownPromise; +} + function _makeJob(fnName) { + if (_shuttingDown && fnName !== 'close') { + throw new Error( + `Rejecting SQL channel job (${fnName}); application is shutting down` + ); + } + _jobCounter += 1; const id = _jobCounter; @@ -219,8 +261,16 @@ function _updateJob(id, data) { function _removeJob(id) { if (_DEBUG) { _jobs[id].complete = true; - } else { - delete _jobs[id]; + return; + } + + delete _jobs[id]; + + if (_shutdownCallback) { + const keys = Object.keys(_jobs); + if (keys.length === 0) { + _shutdownCallback(); + } } } @@ -310,6 +360,14 @@ function keysFromArrayBuffer(keys, data) { // Top-level calls +async function shutdown() { + // Stop accepting new SQL jobs, flush outstanding queue + await _shutdown(); + + // Close database + await close(); +} + // Note: will need to restart the app after calling this, to set up afresh async function close() { await channels.close(); diff --git a/main.js b/main.js index da72f143b..7c5f94523 100644 --- a/main.js +++ b/main.js @@ -329,27 +329,43 @@ function createWindow() { captureClicks(mainWindow); // Emitted when the window is about to be closed. - mainWindow.on('close', e => { + // Note: We do most of our shutdown logic here because all windows are closed by + // Electron before the app quits. + mainWindow.on('close', async e => { + console.log('close event', { + readyForShutdown: mainWindow ? mainWindow.readyForShutdown : null, + shouldQuit: windowState.shouldQuit(), + }); // If the application is terminating, just do the default if ( - windowState.shouldQuit() || config.environment === 'test' || - config.environment === 'test-lib' + config.environment === 'test-lib' || + (mainWindow.readyForShutdown && windowState.shouldQuit()) ) { return; } + // Prevent the shutdown + e.preventDefault(); + mainWindow.hide(); + // On Mac, or on other platforms when the tray icon is in use, the window // should be only hidden, not closed, when the user clicks the close button - if (usingTrayIcon || process.platform === 'darwin') { - e.preventDefault(); - mainWindow.hide(); - + if ( + !windowState.shouldQuit() && + (usingTrayIcon || process.platform === 'darwin') + ) { // toggle the visibility of the show/hide tray icon menu entries if (tray) { tray.updateContextMenu(); } + + return; } + + await requestShutdown(); + mainWindow.readyForShutdown = true; + app.quit(); }); // Emitted when the window is closed. @@ -691,7 +707,51 @@ function setupMenu(options) { Menu.setApplicationMenu(menu); } +async function requestShutdown() { + if (!mainWindow || !mainWindow.webContents) { + return; + } + + console.log('requestShutdown: Requesting close of mainWindow...'); + const request = new Promise((resolve, reject) => { + ipc.once('now-ready-for-shutdown', (_event, error) => { + console.log('requestShutdown: Response received'); + + if (error) { + return reject(error); + } + + return resolve(); + }); + mainWindow.webContents.send('get-ready-for-shutdown'); + + // We'll wait two minutes, then force the app to go down. This can happen if someone + // exits the app before we've set everything up in preload() (so the browser isn't + // yet listening for these events), or if there are a whole lot of stacked-up tasks. + // Note: two minutes is also our timeout for SQL tasks in data.js in the browser. + setTimeout(() => { + console.log( + 'requestShutdown: Response never received; forcing shutdown.' + ); + resolve(); + }, 2 * 60 * 1000); + }); + + try { + await request; + } catch (error) { + console.log( + 'requestShutdown error:', + error && error.stack ? error.stack : error + ); + } +} + app.on('before-quit', () => { + console.log('before-quit event', { + readyForShutdown: mainWindow ? mainWindow.readyForShutdown : null, + shouldQuit: windowState.shouldQuit(), + }); windowState.markShouldQuit(); }); diff --git a/preload.js b/preload.js index f8a2483c7..aec14a957 100644 --- a/preload.js +++ b/preload.js @@ -151,6 +151,25 @@ ipc.on('delete-all-data', () => { } }); +ipc.on('get-ready-for-shutdown', async () => { + const { shutdown } = window.Events; + if (!shutdown) { + window.log.error('preload shutdown handler: shutdown method not found'); + ipc.send('now-ready-for-shutdown'); + return; + } + + try { + await shutdown(); + ipc.send('now-ready-for-shutdown'); + } catch (error) { + ipc.send( + 'now-ready-for-shutdown', + error && error.stack ? error.stack : error + ); + } +}); + function installGetter(name, functionName) { ipc.on(`get-${name}`, async () => { const getFn = window.Events[functionName]; @@ -159,7 +178,10 @@ function installGetter(name, functionName) { try { ipc.send(`get-success-${name}`, null, await getFn()); } catch (error) { - ipc.send(`get-success-${name}`, error); + ipc.send( + `get-success-${name}`, + error && error.stack ? error.stack : error + ); } } });