fix(quit): Destroy hot-loaded windows so they don't block quit - fixes T1282

Summary:
teardown dblite on quit—if it faiels to exit, it blocks the main process from quitting on linux

fix edge case where the ".quit" sql command is queued to run and then doesn't run because closeRequested is true

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Maniphest Tasks: T1282

Differential Revision: https://phab.nylas.com/D1559
This commit is contained in:
Ben Gotow 2015-05-26 14:15:12 -07:00
parent 480b0e799d
commit 337e7a0cd9
4 changed files with 52 additions and 29 deletions

View file

@ -154,6 +154,10 @@ class Application
delete @databases[databasePath]
fs.unlink(databasePath, callback)
teardownAllDatabases: ->
for path, val of @databases
@teardownDatabase(path)
# Creates server to listen for additional atom application launches.
#
# You can run the atom command multiple times, but after the first launch
@ -219,14 +223,11 @@ class Application
@runBenchmarks()
@on 'application:logout', =>
for path, val of @databases
@teardownDatabase(path)
@teardownAllDatabases()
@config.set('nylas', null)
@config.set('edgehill', null)
@on 'application:quit', =>
@quitting = true
app.quit()
@on 'application:quit', => app.quit()
@on 'application:inspect', ({x,y, atomWindow}) ->
atomWindow ?= @windowManager.focusedWindow()
atomWindow?.browserWindow.inspectElement(x, y)
@ -234,9 +235,7 @@ class Application
@on 'application:send-feedback', => @windowManager.sendToMainWindow('send-feedback')
@on 'application:show-main-window', => @windowManager.ensurePrimaryWindowOnscreen()
@on 'application:check-for-update', => @autoUpdateManager.check()
@on 'application:install-update', =>
@quitting = true
@autoUpdateManager.install()
@on 'application:install-update', => @autoUpdateManager.install()
@on 'application:open-dev', =>
@devMode = true
@windowManager.closeMainWindow()
@ -258,10 +257,21 @@ class Application
app.on 'window-all-closed', =>
@windowManager.windowClosedOrHidden()
# Called before the app tries to close any windows.
app.on 'before-quit', =>
# Destroy hot windows so that they can't block the app from quitting.
# (Electron will wait for them to finish loading before quitting.)
@windowManager.unregisterAllHotWindows()
# Allow the main window to be closed.
@quitting = true
# Called after the app has closed all windows.
app.on 'will-quit', =>
@teardownAllDatabases()
@deleteSocketFile()
app.on 'will-exit', =>
@teardownAllDatabases()
@deleteSocketFile()
app.on 'open-file', (event, pathToOpen) ->

View file

@ -168,8 +168,8 @@ class AtomWindow
chosen = dialog.showMessageBox @browserWindow,
type: 'warning'
buttons: ['Close Window', 'Reload', 'Keep It Open']
message: 'The editor has crashed'
detail: 'Please report this issue to https://github.com/atom/atom'
message: 'Nylas Mail has crashed'
detail: 'Please report this issue to us at support@nylas.com.'
switch chosen
when 0 then @browserWindow.destroy()
when 1 then @browserWindow.restart()

View file

@ -39,6 +39,9 @@ class WindowManager
focusedWindow: ->
_.find @_windows, (atomWindow) -> atomWindow.isFocused()
visibleWindows: ->
_.filter @_windows, (atomWindow) -> atomWindow.isVisible()
###
Main Window
@ -199,10 +202,17 @@ class WindowManager
@_replenishHotWindows()
# Immediately close all of the hot windows and reset the replentish queue
# to prevent more from being opened without additional calls to registerHotWindow.
#
# Note: This method calls `browserWindow.destroy()` which closes windows without
# waiting for them to load or firing window lifecycle events. This is necessary
# for the app to quit promptly on Linux. https://phab.nylas.com/T1282
#
unregisterAllHotWindows: ->
for type, {loadedWindows} of @_hotWindows
for win in loadedWindows
win.close()
win.browserWindow.destroy()
@_replenishQueue = []
@_hotWindows = {}
@ -349,10 +359,7 @@ class WindowManager
windowClosedOrHidden: ->
if process.platform in ['win32', 'linux']
visible = false
visible ||= window.isVisible() for window in @_windows
if visible is false
global.application.quitting = true
if @visibleWindows().length is 0
# Quitting the app from within a window event handler causes
# an assertion error. Wait a moment.
_.defer -> app.quit()

View file

@ -161,7 +161,7 @@ var
* db.query('SELECT table.a, table.other FROM table', ['a', 'b']);
* [{a:'first value', b:'second'},{a:'row2 value', b:'row2'}]
*
*
*
* db.query('SELECT table.a, table.other FROM table', ['a', 'b']);
* [{a:'first value', b:'second'},{a:'row2 value', b:'row2'}]
* callback:Function
@ -215,7 +215,8 @@ function dblite() {
// accordingly. As simple as that ^_^
queue = [],
// set as true only once db.close() has been called
notWorking = false,
closeRequested = false,
closed = false,
// marks the shell busy or not
busy = false,
// tells if current output needs to be processed
@ -412,20 +413,25 @@ function dblite() {
// property is explicitly set to false
// it keeps running queries no matter what
self.ignoreErrors = false;
// - - - - - - - - - - - - - - - - - - - -
// safely closes the process
// will emit 'close' once done
self.close = function() {
// close can happen only once
if (!notWorking) {
// this should gently terminate the program
// only once everything scheduled has been completed
self.query('.exit');
notWorking = true;
// the hardly killed version was like this:
// program.stdin.end();
// program.kill();
if (!closeRequested) {
var kill = function() {
closed = true;
program.stdin.end();
program.kill();
}
// send the .exit pragma to sqlite, and kill the process once the query
// executes. If we don't hear back for 1000msec, force kill sqlite.
// This is necessary because on Linux the process can be left open forever.
//
self.query('.exit', null, null, kill);
closeRequested = true;
}
};
@ -465,10 +471,10 @@ function dblite() {
// main logic/method/entry point
self.query = function(string, params, fields, callback) {
// notWorking is set once .close() has been called
// closed is set once .close() has been called and run.
// it does not make sense to execute anything after
// the program is being closed
if (notWorking) return onerror('closing'), self;
if (closed) return onerror('closed'), self;
// if something is still going on in the sqlite3 shell
// the progcess is flagged as busy. Just queue other operations
if (busy) return queue.push(arguments), self;
@ -903,4 +909,4 @@ db.query('CREATE TABLE test (key INTEGER PRIMARY KEY, value TEXT)') && undefined
db.query('INSERT INTO test VALUES(null, "asd")') && undefined;
db.query('SELECT * FROM test') && undefined;
// db.close();
*/
*/