From ccab3e846cc76d09c233d8ef90783fa899ed723e Mon Sep 17 00:00:00 2001 From: Jan Sprinz Date: Tue, 29 Oct 2019 18:53:50 +0100 Subject: [PATCH 1/2] Revert "Make sure adb does not live on as a zombie process. EXTERMINATE!" This reverts commit dc2b56ca50c96e20e5f8d8487008ebd42627d563. --- package-lock.json | 69 ----------------------------------------------- package.json | 1 - src/main.js | 8 +++--- 3 files changed, 3 insertions(+), 75 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1a18fc8f..55800758 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1785,11 +1785,6 @@ } } }, - "duplexer": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/duplexer/-/duplexer-0.1.1.tgz", - "integrity": "sha1-rOb/gIwc5mtX0ev5eXessCM0z8E=" - }, "duplexer2": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/duplexer2/-/duplexer2-0.1.4.tgz", @@ -2540,20 +2535,6 @@ "resolved": "https://registry.npmjs.org/esutils/-/esutils-2.0.2.tgz", "integrity": "sha1-Cr9PHKpbyx96nYrMbepPqqBLrJs=" }, - "event-stream": { - "version": "3.3.4", - "resolved": "https://registry.npmjs.org/event-stream/-/event-stream-3.3.4.tgz", - "integrity": "sha1-SrTJoPWlTbkzi0w02Gv86PSzVXE=", - "requires": { - "duplexer": "~0.1.1", - "from": "~0", - "map-stream": "~0.1.0", - "pause-stream": "0.0.11", - "split": "0.3", - "stream-combiner": "~0.0.4", - "through": "~2.3.1" - } - }, "eventie": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/eventie/-/eventie-1.0.6.tgz", @@ -2864,11 +2845,6 @@ "mime-types": "^2.1.12" } }, - "from": { - "version": "0.1.7", - "resolved": "https://registry.npmjs.org/from/-/from-0.1.7.tgz", - "integrity": "sha1-g8YK/Fi5xWmXAH7Rp2izqzA6RP4=" - }, "from2": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/from2/-/from2-2.3.0.tgz", @@ -4204,11 +4180,6 @@ "integrity": "sha1-2TPOuSBdgr3PSIb2dCvcK03qFG0=", "dev": true }, - "map-stream": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/map-stream/-/map-stream-0.1.0.tgz", - "integrity": "sha1-5WqpTEyAVaFkBKBnS3jyFffI4ZQ=" - }, "mem": { "version": "4.3.0", "resolved": "https://registry.npmjs.org/mem/-/mem-4.3.0.tgz", @@ -5070,14 +5041,6 @@ "resolved": "https://registry.npmjs.org/pathval/-/pathval-1.1.0.tgz", "integrity": "sha1-uULm1L3mUwBe9rcTYd74cn0GReA=" }, - "pause-stream": { - "version": "0.0.11", - "resolved": "https://registry.npmjs.org/pause-stream/-/pause-stream-0.0.11.tgz", - "integrity": "sha1-/lo0sMvOErWqaitAPuLnO2AvFEU=", - "requires": { - "through": "~2.3" - } - }, "pend": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz", @@ -5224,14 +5187,6 @@ "resolved": "https://registry.npmjs.org/proto-list/-/proto-list-1.2.4.tgz", "integrity": "sha1-IS1b/hMYMGpCD2QCuOJv85ZHqEk=" }, - "ps-tree": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/ps-tree/-/ps-tree-1.2.0.tgz", - "integrity": "sha512-0VnamPPYHl4uaU/nSFeZZpR21QAWRz+sRv4iW9+v/GS/J5U5iZB5BNN6J0RMoOvdx2gWM2+ZFMIm58q24e4UYA==", - "requires": { - "event-stream": "=3.3.4" - } - }, "pseudomap": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/pseudomap/-/pseudomap-1.0.2.tgz", @@ -5963,14 +5918,6 @@ "integrity": "sha1-mHbb0qFp0xFUAtSObqYynIgWpQ0=", "dev": true }, - "split": { - "version": "0.3.3", - "resolved": "https://registry.npmjs.org/split/-/split-0.3.3.tgz", - "integrity": "sha1-zQ7qXmOiEd//frDwkcQTPi0N0o8=", - "requires": { - "through": "2" - } - }, "sprintf-js": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", @@ -6003,14 +5950,6 @@ "integrity": "sha512-QjMLR0A3WwFY2aZdV0okfFEJB5TRjkggXZjxP3A1RsWsNHNu3YPv8btmtc6iCFZ0Rul3FE93OYogvhOUClU+ng==", "dev": true }, - "stream-combiner": { - "version": "0.0.4", - "resolved": "https://registry.npmjs.org/stream-combiner/-/stream-combiner-0.0.4.tgz", - "integrity": "sha1-TV5DPBhSYd3mI8o/RMWGvPXErRQ=", - "requires": { - "duplexer": "~0.1.1" - } - }, "strict-uri-encode": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/strict-uri-encode/-/strict-uri-encode-1.1.0.tgz", @@ -6263,14 +6202,6 @@ "execa": "^0.7.0" } }, - "terminate": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/terminate/-/terminate-2.1.2.tgz", - "integrity": "sha512-ltKc9MkgcRe7gzD7XSttHCF1feKM1pTkCdb58jFVWk1efPN9JIk/BHSlOaYF+hCcWoubeJQ8C8Phb0++fa6iNQ==", - "requires": { - "ps-tree": "^1.1.1" - } - }, "test-exclude": { "version": "5.2.3", "resolved": "https://registry.npmjs.org/test-exclude/-/test-exclude-5.2.3.tgz", diff --git a/package.json b/package.json index 6043737b..41d68bf2 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,6 @@ "promise-android-tools": "^1.0.6", "request": "^2.79.0", "system-image-node-module": "^1.0.10", - "terminate": "^2.1.2", "ubports-api-node-module": "^2.0.1", "winston": "^3.1.2" } diff --git a/src/main.js b/src/main.js index 05360bd5..e24ce835 100755 --- a/src/main.js +++ b/src/main.js @@ -18,9 +18,8 @@ */ const cli = require("commander"); -const electron = require("electron"); -const electronPug = require("electron-pug"); -const terminate = require("terminate"); +const electron = require('electron'); +const electronPug = require('electron-pug'); const app = electron.app; const BrowserWindow = electron.BrowserWindow; @@ -436,8 +435,7 @@ app.on("window-all-closed", function() { if (process.platform !== "darwin") { setTimeout(() => { app.quit(); - terminate(process.pid, console.error); - }, 1000); + }, 50); } }); From 58ffb0cb4912797e406bf3fbdd2659202dd5148e Mon Sep 17 00:00:00 2001 From: Jan Sprinz Date: Wed, 30 Oct 2019 12:48:01 +0100 Subject: [PATCH 2/2] Avoid zombie processes, fixes #978 --- package-lock.json | 63 ++++++++++++++++++++++++++++++++++++++++++++++- package.json | 1 + src/devices.js | 12 ++++++--- src/main.js | 34 ++++++++++++------------- src/utils.js | 26 +++++++++++++++++-- 5 files changed, 112 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index 55800758..b6de4e0e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "ubports-installer", - "version": "0.4.1-beta", + "version": "0.4.2-beta", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -1785,6 +1785,11 @@ } } }, + "duplexer": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/duplexer/-/duplexer-0.1.1.tgz", + "integrity": "sha1-rOb/gIwc5mtX0ev5eXessCM0z8E=" + }, "duplexer2": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/duplexer2/-/duplexer2-0.1.4.tgz", @@ -2535,6 +2540,20 @@ "resolved": "https://registry.npmjs.org/esutils/-/esutils-2.0.2.tgz", "integrity": "sha1-Cr9PHKpbyx96nYrMbepPqqBLrJs=" }, + "event-stream": { + "version": "3.3.4", + "resolved": "https://registry.npmjs.org/event-stream/-/event-stream-3.3.4.tgz", + "integrity": "sha1-SrTJoPWlTbkzi0w02Gv86PSzVXE=", + "requires": { + "duplexer": "~0.1.1", + "from": "~0", + "map-stream": "~0.1.0", + "pause-stream": "0.0.11", + "split": "0.3", + "stream-combiner": "~0.0.4", + "through": "~2.3.1" + } + }, "eventie": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/eventie/-/eventie-1.0.6.tgz", @@ -2845,6 +2864,11 @@ "mime-types": "^2.1.12" } }, + "from": { + "version": "0.1.7", + "resolved": "https://registry.npmjs.org/from/-/from-0.1.7.tgz", + "integrity": "sha1-g8YK/Fi5xWmXAH7Rp2izqzA6RP4=" + }, "from2": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/from2/-/from2-2.3.0.tgz", @@ -4180,6 +4204,11 @@ "integrity": "sha1-2TPOuSBdgr3PSIb2dCvcK03qFG0=", "dev": true }, + "map-stream": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/map-stream/-/map-stream-0.1.0.tgz", + "integrity": "sha1-5WqpTEyAVaFkBKBnS3jyFffI4ZQ=" + }, "mem": { "version": "4.3.0", "resolved": "https://registry.npmjs.org/mem/-/mem-4.3.0.tgz", @@ -5041,6 +5070,14 @@ "resolved": "https://registry.npmjs.org/pathval/-/pathval-1.1.0.tgz", "integrity": "sha1-uULm1L3mUwBe9rcTYd74cn0GReA=" }, + "pause-stream": { + "version": "0.0.11", + "resolved": "https://registry.npmjs.org/pause-stream/-/pause-stream-0.0.11.tgz", + "integrity": "sha1-/lo0sMvOErWqaitAPuLnO2AvFEU=", + "requires": { + "through": "~2.3" + } + }, "pend": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz", @@ -5187,6 +5224,14 @@ "resolved": "https://registry.npmjs.org/proto-list/-/proto-list-1.2.4.tgz", "integrity": "sha1-IS1b/hMYMGpCD2QCuOJv85ZHqEk=" }, + "ps-tree": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/ps-tree/-/ps-tree-1.2.0.tgz", + "integrity": "sha512-0VnamPPYHl4uaU/nSFeZZpR21QAWRz+sRv4iW9+v/GS/J5U5iZB5BNN6J0RMoOvdx2gWM2+ZFMIm58q24e4UYA==", + "requires": { + "event-stream": "=3.3.4" + } + }, "pseudomap": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/pseudomap/-/pseudomap-1.0.2.tgz", @@ -5918,6 +5963,14 @@ "integrity": "sha1-mHbb0qFp0xFUAtSObqYynIgWpQ0=", "dev": true }, + "split": { + "version": "0.3.3", + "resolved": "https://registry.npmjs.org/split/-/split-0.3.3.tgz", + "integrity": "sha1-zQ7qXmOiEd//frDwkcQTPi0N0o8=", + "requires": { + "through": "2" + } + }, "sprintf-js": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", @@ -5950,6 +6003,14 @@ "integrity": "sha512-QjMLR0A3WwFY2aZdV0okfFEJB5TRjkggXZjxP3A1RsWsNHNu3YPv8btmtc6iCFZ0Rul3FE93OYogvhOUClU+ng==", "dev": true }, + "stream-combiner": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/stream-combiner/-/stream-combiner-0.0.4.tgz", + "integrity": "sha1-TV5DPBhSYd3mI8o/RMWGvPXErRQ=", + "requires": { + "duplexer": "~0.1.1" + } + }, "strict-uri-encode": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/strict-uri-encode/-/strict-uri-encode-1.1.0.tgz", diff --git a/package.json b/package.json index 41d68bf2..3d2b7926 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "download": "^7.1.0", "electron-prompt": "^1.4.0", "popper.js": "^1.14.3", + "ps-tree": "^1.2.0", "checksum": "^0.1.1", "commander": "^2.9.0", "electron-open-link-in-browser": "^1.0.2", diff --git a/src/devices.js b/src/devices.js index 16802b0b..b1b15da4 100644 --- a/src/devices.js +++ b/src/devices.js @@ -283,6 +283,8 @@ function assembleInstallSteps(steps) { error.includes("No such device") ) { mainEvent.emit("user:connection-lost", runStep); + } else if (error.includes("Killed")) { + reject(); // Used for exiting the installer } else { utils.errorToUser(error, step.type); } @@ -316,10 +318,12 @@ function assembleInstallSteps(steps) { function install(steps) { var installPromises = assembleInstallSteps(steps); // Actually run the steps - installPromises.reduce( - (promiseChain, currentFunction) => promiseChain.then(currentFunction), - Promise.resolve() - ); + installPromises + .reduce( + (promiseChain, currentFunction) => promiseChain.then(currentFunction), + Promise.resolve() + ) + .catch(() => {}); // errors can be ignored here, since this is exclusively used for killing the promise chain } module.exports = { diff --git a/src/main.js b/src/main.js index e24ce835..b53d2125 100755 --- a/src/main.js +++ b/src/main.js @@ -18,8 +18,8 @@ */ const cli = require("commander"); -const electron = require('electron'); -const electronPug = require('electron-pug'); +const electron = require("electron"); +const electronPug = require("electron-pug"); const app = electron.app; const BrowserWindow = electron.BrowserWindow; @@ -187,9 +187,8 @@ ipcMain.on("option", (event, targetVar, value) => { mainEvent.on("user:error", err => { try { if (mainWindow) mainWindow.webContents.send("user:error", err); - else utils.die(err); + else process.exit(1); } catch (e) { - utils.log.error(e); process.exit(1); } }); @@ -358,11 +357,6 @@ function createWindow() { global.packageInfo.version + "!" ); - utils.log.info( - "This is " + - (global.packageInfo.updateAvailable ? "not " : "") + - "the latest stable version!" - ); mainWindow = new BrowserWindow({ width: cli.cli ? 0 : cli.debug ? 1600 : 800, height: cli.cli ? 0 : 600, @@ -380,11 +374,15 @@ function createWindow() { devices.waitForDevice(); } }) - .catch(e => utils.errorToUser(e, "Failed to start adb server")); + .catch(e => { + if (!e.includes("Killed")) + utils.errorToUser(e, "Failed to start adb server"); + }); api .getDeviceSelects() .then(out => { - mainWindow.webContents.send("device:wait:device-selects-ready", out); + if (mainWindow) + mainWindow.webContents.send("device:wait:device-selects-ready", out); }) .catch(e => { utils.log.error("getDeviceSelects error: " + e); @@ -403,9 +401,7 @@ function createWindow() { ); mainWindow.webContents.send("user:update-available"); }) - .catch(() => { - utils.log.debug("This is the latest version."); - }); + .catch(() => {}); // Ignore errors, since this is non-essential }); mainWindow.loadURL( @@ -430,12 +426,16 @@ function createWindow() { app.on("ready", createWindow); app.on("window-all-closed", function() { - adb.killServer().catch(); - utils.log.info("Good bye!"); + adb + .killServer() + .then(utils.killSubprocesses) + .catch(utils.killSubprocesses); if (process.platform !== "darwin") { + utils.log.info("Good bye!"); setTimeout(() => { app.quit(); - }, 50); + process.exit(0); + }, 1000); } }); diff --git a/src/utils.js b/src/utils.js index 21fbb32c..9a4e8b55 100644 --- a/src/utils.js +++ b/src/utils.js @@ -26,6 +26,7 @@ const checksum = require("checksum"); const mkdirp = require("mkdirp"); const cp = require("child_process"); const getos = require("getos"); +const psTree = require("ps-tree"); global.packageInfo = require("../package.json"); if (!fs.existsSync(getUbuntuTouchDir())) { @@ -229,12 +230,32 @@ let toolpath = global.packageInfo.package platforms[os.platform()] ) : path.join(__dirname, "..", "platform-tools", platforms[os.platform()]); +let processes = []; function execTool(tool, args, callback) { - exec( + let pid = exec( [path.join(toolpath, tool)].concat(args).join(" "), - { options: { maxBuffer: 1024 * 1024 * 2 } }, + { + maxBuffer: 1024 * 1024 * 2 + }, callback ); + processes.push(pid); + pid.on("exit", () => { + processes.splice(processes.indexOf(pid), 1); + }); +} + +// Since child_process.exec spins up a shell on posix, simply killing the process itself will orphan its children, who then will be adopted by pid 1 and continue running as zombie processes until the end of time. +function killSubprocesses() { + if (process.platform === "win32") { + processes.forEach(child => child.kill()); + } else { + processes.forEach(pid => { + psTree(pid.pid, function(err, children) { + cp.spawn("kill", ["-9"].concat(children.map(p => p.PID))); + }); + }); + } } function isSnap() { @@ -370,6 +391,7 @@ module.exports = { log: log, isSnap: isSnap, execTool: execTool, + killSubprocesses: killSubprocesses, getUbuntuTouchDir: getUbuntuTouchDir, sendBugReport: sendBugReport, getUpdateAvailable: getUpdateAvailable,