From e26c85db464f7b4cd39e861a39a2b096d505ef1b Mon Sep 17 00:00:00 2001 From: meriadec Date: Mon, 25 Jun 2018 23:27:11 +0200 Subject: [PATCH] Fix zombie processes by hardly terminate them on window close To be improved: this commits introduce regression on OSX behaviour when hiding the window: before it was keeping the app opened, and auto locking it. Now it kills it :D I honestly think it's worth it regarding the problem zombies process causes (LOCK on sqlite...). --- src/main/app.js | 37 +++++++++++++++++++++++-------------- src/main/bridge.js | 2 ++ src/main/terminator.js | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 src/main/terminator.js diff --git a/src/main/app.js b/src/main/app.js index 65a224ac..73633ebd 100644 --- a/src/main/app.js +++ b/src/main/app.js @@ -7,12 +7,17 @@ import { MIN_HEIGHT, MIN_WIDTH } from 'config/constants' import menu from 'main/menu' import db from 'helpers/db' +import { setMainProcessPID, terminateAllTheThings } from './terminator' + +setMainProcessPID(process.pid) + // necessary to prevent win from being garbage collected let mainWindow = null export const getMainWindow = () => mainWindow -let forceClose = false +// TODO put back OSX close behavior +// let forceClose = false const { UPGRADE_EXTENSIONS, ELECTRON_WEBPACK_WDS_PORT, DEV_TOOLS, DEV_TOOLS_MODE } = process.env @@ -27,15 +32,16 @@ const getWindowPosition = (height, width, display = screen.getPrimaryDisplay()) } } -const handleCloseWindow = w => e => { - if (!forceClose) { - e.preventDefault() - w.webContents.send('lock') - if (w !== null) { - w.hide() - } - } -} +// TODO put back OSX close behavior +// const handleCloseWindow = w => e => { +// if (!forceClose) { +// e.preventDefault() +// w.webContents.send('lock') +// if (w !== null) { +// w.hide() +// } +// } +// } const getDefaultUrl = () => __DEV__ ? `http://localhost:${ELECTRON_WEBPACK_WDS_PORT || ''}` : `file://${__dirname}/index.html` @@ -107,7 +113,9 @@ function createMainWindow() { window.loadURL(url) - window.on('close', handleCloseWindow(window)) + // TODO put back OSX close behavior + // window.on('close', handleCloseWindow(window)) + window.on('close', terminateAllTheThings) window.on('ready-to-show', () => { window.show() @@ -126,9 +134,10 @@ function createMainWindow() { return window } -app.on('before-quit', () => { - forceClose = true -}) +// TODO put back OSX close behavior +// app.on('before-quit', () => { +// forceClose = true +// }) app.on('window-all-closed', () => { // On macOS it is common for applications to stay open diff --git a/src/main/bridge.js b/src/main/bridge.js index 79323d2e..5ed618b2 100644 --- a/src/main/bridge.js +++ b/src/main/bridge.js @@ -11,6 +11,7 @@ import sentry from 'sentry/node' import user from 'helpers/user' import setupAutoUpdater, { quitAndInstall } from './autoUpdate' +import { setInternalProcessPID } from './terminator' import { getMainWindow } from './app' @@ -49,6 +50,7 @@ const bootInternalProcess = () => { SENTRY_USER_ID: userId, }, }) + setInternalProcessPID(internalProcess.pid) internalProcess.on('message', handleGlobalInternalMessage) internalProcess.on('exit', handleExit) } diff --git a/src/main/terminator.js b/src/main/terminator.js new file mode 100644 index 00000000..8bc0f6cc --- /dev/null +++ b/src/main/terminator.js @@ -0,0 +1,32 @@ +// @flow + +// <((((((\\\ +// / . }\ +// ;--..--._|} +// (\ '--/\--' ) +// DISCLAIMER \\ | '-' :'| +// This is a dirty hack \\ . -==- .-| +// \\ \.__.' \--._ +// [\\ __.--| // _/'--. +// \ \\ .'-._ ('-----'/ __/ \ +// \ \\ / __>| | '--. | +// \ \\ | \ | / / / +// \ '\ / \ | | _/ / +// \ \ \ | | / / +// \ \ \ / + +let MAIN_PROCESS_PID: ?number = null +let INTERNAL_PROCESS_PID: ?number = null + +function kill(processType, pid) { + console.log(`-> Killing ${processType} process ${pid}`) // eslint-disable-line no-console + process.kill(pid, 'SIGTERM') +} + +exports.setMainProcessPID = (pid: number) => (MAIN_PROCESS_PID = pid) +exports.setInternalProcessPID = (pid: number) => (INTERNAL_PROCESS_PID = pid) + +exports.terminateAllTheThings = () => { + if (INTERNAL_PROCESS_PID) kill('internal', INTERNAL_PROCESS_PID) + if (MAIN_PROCESS_PID) kill('main', MAIN_PROCESS_PID) +}