Make upgrade process better at killing old daemons

- Manually call xdg-open instead of using shell.openItem(), which
   doesn't reliably work from the main process
 - If there's a connection error or timeout when asking the daemon
   to close, fall back on force killing
This commit is contained in:
Alex Liebowitz 2017-03-22 06:47:44 -04:00
parent 7d32c8e98f
commit 4958f9decf
2 changed files with 49 additions and 14 deletions

View file

@ -1,9 +1,10 @@
const {app, BrowserWindow, ipcMain, shell} = require('electron');
const {app, BrowserWindow, ipcMain} = require('electron');
const path = require('path');
const jayson = require('jayson');
// tree-kill has better cross-platform handling of
// killing a process. child-process.kill was unreliable
const kill = require('tree-kill');
const child_process = require('child_process');
let client = jayson.client.http('http://localhost:5279/lbryapi');
@ -37,7 +38,7 @@ function launchDaemon() {
executable = path.join(__dirname, 'dist', 'lbrynet-daemon');
}
console.log('Launching daemon: ' + executable)
subpy = require('child_process').spawn(executable)
subpy = child_process.spawn(executable)
// Need to handle the data event instead of attaching to
// process.stdout because the latter doesn't work. I believe on
// windows it buffers stdout and we don't get any meaningful output
@ -86,6 +87,31 @@ function launchDaemonIfNotRunning() {
);
}
/*
* Last resort for killing unresponsive daemon instances.
* Looks for any processes called "lbrynet-daemon" and
* tries to force kill them.
*/
function forceKillAllDaemons() {
console.log("Attempting to force kill any running lbrynet-daemon instances...");
const fgrepOut = child_process.spawnSync('pgrep', ['-x', 'lbrynet-daemon'], {encoding: 'utf8'}).stdout;
const daemonPids = fgrepOut.split(/[^\d]+/).filter((pid) => pid);
if (!daemonPids) {
console.log('No lbrynet-daemon found running.');
} else {
console.log(`Found ${daemonPids.length} running daemon instances. Attempting to force kill...`);
for (const pid of daemonPids) {
kill(pid, 'SIGKILL', (err) => {
if (err) {
console.log(`Failed to force kill running daemon with pid ${pid}. Error message: ${err.message}`);
}
});
}
}
}
// Quit when all windows are closed.
app.on('window-all-closed', () => {
@ -122,10 +148,17 @@ function shutdownDaemon(evenIfNotStartedByApp = false) {
console.log('Killed lbrynet-daemon process');
});
} else if (evenIfNotStartedByApp) {
console.log('Killing lbrynet-daemon, even though app did not start it');
client.request('daemon_stop', []);
// TODO: If the daemon errors or times out when we make this request, find
// the process and force quit it.
console.log('Stopping lbrynet-daemon, even though app did not start it');
client.request('daemon_stop', [], (err, res) => {
if (err) {
// We could get an error because the daemon is already stopped (good)
// or because it's running but not responding properly (bad).
// So try to force kill any daemons that are still running.
console.log('received error when stopping lbrynet-daemon. Error message: {err.message}');
forceKillAllDaemons();
}
});
} else {
console.log('Not killing lbrynet-daemon because app did not start it')
}
@ -144,7 +177,10 @@ function shutdown() {
function upgrade(event, installerPath) {
app.on('quit', () => {
shell.openItem(installerPath);
// shell.openItem doesn't reliably work from the app process, so run xdg-open directly
child_process.spawn('xdg-open', [installerPath], {
stdio: 'ignore',
});
});
if (win) {
win.loadURL(`file://${__dirname}/dist/upgrade.html`);
@ -160,4 +196,4 @@ function upgrade(event, installerPath) {
ipcMain.on('upgrade', upgrade);
ipcMain.on('shutdown', shutdown);
ipcMain.on('shutdown', shutdown);

View file

@ -27,6 +27,7 @@ const {download} = remote.require('electron-dl');
const os = require('os');
const path = require('path');
const app = require('electron').remote.app;
const fs = remote.require('fs');
var App = React.createClass({
@ -137,16 +138,14 @@ var App = React.createClass({
});
},
handleUpgradeClicked: function() {
// TODO: create a callback for onProgress and have the UI
// show download progress
// TODO: calling lbry.stop() ends up displaying the "daemon
// unexpectedly stopped" page. Have a better way of shutting down
let dir = app.getPath('temp');
// Make a new directory within temp directory so the filename is guaranteed to be available
const dir = fs.mkdtempSync(app.getPath('temp') + require('path').sep);
let options = {
onProgress: (p) => this.setState({downloadProgress: Math.round(p * 100)}),
directory: dir,
};
download(remote.getCurrentWindow(), this.state.updateUrl, options)
download(remote.getCurrentWindow(), this.getUpdateUrl(), options)
.then(downloadItem => {
/**
* TODO: get the download path directly from the download object. It should just be