Express process is not killed if app closes before window has created

[orensharon]

After starting electron app, In case: Express process has spawned, Browser-window hasn't created yet and app has closed (signal or error) then node-express process is not been killed (I can see the 'dangling' node process in Task-Manager)

This is how I'm starting the express process (not detached):

serverProcess = spawn(nodeDotExe, args, {
    cwd: app.getAppPath(),
    shell: true,
    env: process.env,
    windowsHide: true
})

Killing the app (just like the rewrite):

app.on("window-all-closed", function () {
    console.log('window-all-closed')
    if (process.platform !== "darwin") {
        app.quit();
    }
});

Catching process events (exit, SIGINT, SIGUSR1, SIGUSR2) didn't help.

[frankhale]

Referencing issue https://github.com/frankhale/electron-with-express/issues/15 as well

[frankhale]

Processes exiting cleanly is being addressed in the rewrite branch. I'm going to close this as a duplicate and in favor of the issue that tracks the rewrite: https://github.com/frankhale/electron-with-express/issues/21

Additionally the process issue has been really tricky to figure out and is currently my biggest headache. If you have ideas on how to resolve this I am all ears. All future updates to this issue will happen in the issue that tracks the rewrite branch which I've mentioned above.

[orensharon]

I did some checks. Tried to put redirectOutput(..) in comment and the child process killed. It might be ChildProcess.stdio related issue.

According this thread https://stackoverflow.com/questions/32705857/cant-kill-child-process-on-windows :

... the stdio stream of the parent process is still shared with the other processes in the tree. So the parent process is waiting for the stream to be closed.

I will continue my tests.

[orensharon]

Update: I discovered that option shell: true, wraps the child process by a new Windows Command Processor (you can see it in task-manager). For some reason, the kill() command in main.js, causes the child (node process) to be detached (instead of to being killed) from the wrapper process.

I read about fork, it seems to have better interface for the this use case:

The child_process.fork() method is a special case of child_process.spawn() used specifically to spawn new Node.js processes.

Now my code looks like this:

const nodeDotExe = `${config.nodeExecPath}\\node.exe`
const args = ['electron']

serverProcess = fork("./express-app/index.js", args, {
    cwd: app.getAppPath(),
    execPath: nodeDotExe,
    stdio: 'pipe',
    env: {
        RESOURCES_PATH: config.resourcesFolder,
        USER_DATA: config.userData,
        SERVER_PORT: expressAppConfig.port
    }
})

I hope I didn't miss anything, but now it seems that the problem is gone.

[frankhale]

Thanks @orensharon for your investigative work. In my rewrite branch I've had good success with how I'm doing it now. The process is dying nicely when I exit Electron (on Windows). Additionally I'm stood up some socket.io connections for communicating with server and electron main process. There is still some work to do but it's going nicely so far.

[orensharon]

I hope your solution won't be overkill.. Did you see what I wrote about fork? it's very simple solution ...

[frankhale]

I’m just using spawn and a module to kill process tree on electron close

On Sat, May 18, 2019 at 10:58 Sharon Oren notifications@github.com wrote:

I hope your solution won't be overkill.. Did you see what I wrote about fork? it's very simple solution ...

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/frankhale/electron-with-express/issues/22?email_source=notifications&email_token=AABONSQ5VAWZPARFZK5W7F3PWAKSNA5CNFSM4HK22WFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVWQFPA#issuecomment-493683388, or mute the thread https://github.com/notifications/unsubscribe-auth/AABONSWFWTPHO72PD5BWQDLPWAKSNANCNFSM4HK22WFA .

[frankhale]

I see your code using fork. I will take a deeper look at it. Thank you for sharing this!!!!