Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

The watch tasks seems to be running in the background even after the app is closed #12

Closed
floydpink opened this issue Jun 22, 2015 · 23 comments

Comments

@floydpink
Copy link

Try these steps to reproduce this issue:

  1. Clone the repo
  2. Run npm install at the root of the repo
  3. Run npm start to start the electron app
  4. Close the app - the terminal window will also show the prompt (giving the indication that all the background tasks are terminated)
  5. Make a trivial whitespace change to /app/hello_world/hello_universe.js and save
  6. Notice that the terminal window will start the transpile-watch task
$ [12:37:26] Starting 'transpile-watch'...
[12:37:26] Finished 'transpile-watch' after 47 ms

Testing this with the latest commit as of now on a Mac, with node and npm versions as below:

$ node -v
v0.12.4

$ npm -v
2.11.3
@carlosperate
Copy link
Contributor

I can confirm the same under Windows 8.1.

@szwacz
Copy link
Owner

szwacz commented Jun 22, 2015

Ok, so it wasn't only my machine :) It happened randomly and rarely so I assumed this is some funkiness on my system.
Added explicit kill to on main process exit (9aefea2). How about now?

@floydpink
Copy link
Author

👍 - that seems to have fixed it

@carlosperate
Copy link
Contributor

Sorry I can't say the same, still experiencing the same transpile-watch issue.

@szwacz
Copy link
Owner

szwacz commented Jun 23, 2015

@carlosperate are you sure you've killed all zombie processes of gulp watch started previously before trying this?

@carlosperate
Copy link
Contributor

Yeah, I just tried it on a fresh restart with a new clone of master and still the same.
I should also add that when the I close the application I also get the following errors:

[1988:0623/205934:ERROR:ipc_channel_win.cc(414)] pipe error: 232
[1988:0623/205934:ERROR:ipc_channel_win.cc(138)] pipe error: 109

I've experience both these issues from the beginning (a bit longer than a week ago is when I first used it), and it has not been intermittent.

Testing on Windows 8.1 64 bit, running node 0.12.4 32 bit, and npm 2.10.1.

@szwacz
Copy link
Owner

szwacz commented Jun 24, 2015

Hah, it appears killing child process on Windows is harder than one might think. Could you try with 125b458 ?

@carlosperate
Copy link
Contributor

Yeah, that did the trick.
Although now I am only getting one pipe error when the window is closed, should I open a different issue for that?

[2360:0626/012206:ERROR:ipc_channel_win.cc(138)] pipe error: 109

@szwacz
Copy link
Owner

szwacz commented Jun 26, 2015

Great!

I have no idea what this pipe error is, and it's generated from low level electron stuff (not a JS error), so I don't know if there is something I can do with it.

@szwacz szwacz closed this as completed Jun 26, 2015
@carlosperate
Copy link
Contributor

Have you seen such pip error on your Windows testing? If so, could you raise a bug report in the appropriate channel? I have no idea how the software stack is organised, simple google search about it usually points to chromium build errors.

@pkunze
Copy link
Contributor

pkunze commented Feb 23, 2016

I think this issue should be reopened. I am able to reproduce it with a fresh install of the boilerplate. I noticed it because after I close the electron-window the console does return and prints out right afterwards.

[08:25:08] Starting gulpfile /path/to/gulpfile.js
[08:25:08] Starting 'watch'...

again (after it already retorned to the working directory. See Screenshot below):

gulp-watch-after-quit

After i noticed this, I tried to reproduce the issue using the steps provided by @floydpink wich works sometimes.

Also, the build takes longer everytime I compile again. Also, after using npm start a few times, the cpu load is at a steady pace of 100% on every (Windows-)maschine I try it with. This can be solved temporarely by closing the used cmd window and using a new one, but the process slowly starts again.

Am I doing something wrong?

@black-snow
Copy link
Contributor

Nah, it's the same here, too. I just got used to it. Gotta hit crtl + c a couple times when I wanna stop everything.

@carlosperate
Copy link
Contributor

I've experienced the same.

@szwacz szwacz reopened this Feb 23, 2016
@szwacz
Copy link
Owner

szwacz commented Feb 23, 2016

Why noone told then? :)
I don't know why killing child processes on window is so difficult. Will into it in near future. All helping hands are welcome!

@black-snow
Copy link
Contributor

While it's annoying you also get used to it pretty fast :D

I recently had quite the same issue in one of my projects. I didn't find any satisfying solution. Tried pretty much any of these answers: http://stackoverflow.com/questions/23706055/why-can-i-not-kill-my-child-process-in-nodejs-on-windows
See also https://www.exratione.com/2013/05/die-child-process-die/ and nodejs/node-v0.x-archive#1811

Maybe this will do the trick: https://www.npmjs.com/package/tree-kill

@pkunze
Copy link
Contributor

pkunze commented Feb 24, 2016

I think i've localized the possible issue:

it's in tasks/start.js:

I added some console-logs to make my point clear. see the attached console-screenshot* for an example...

runGulpWatch respawns the process when watch is closed:

var runGulpWatch = function () {
    console.log('[runGulpWatch] Im getting spawned now')

    watch = childProcess.spawn(utils.spawnablePath(gulpPath), [
        'watch',
        '--env=' + utils.getEnvName(),
        '--color'
    ], {
        stdio: 'inherit'
    });

    watch.on('close', function (code) {
        // Gulp watch exits when error occured during build.
        // Just respawn it then.
        console.log('[runGulpWatch] Im restarting my self now')
        runGulpWatch();
    });
};

which is also triggered when the app exits by doing

var runApp = function () {
    var app = childProcess.spawn(electron, ['./build'], {
        stdio: 'inherit'
    });

    app.on('close', function (code) {
        // User closed the app. Kill the host process.
        console.log('[runApp] Im killing gulps watch task right now ;)')
        kill(watch.pid, 'SIGKILL', function () {            
            process.exit();
        });
    });
};

which seems to respawn the watch task (at least on windows):

gulp-watch-after-quit-2

So from my point of view, the automatic respawn of watch causes the problem.

jimbuck pushed a commit to jimbuck/electron-boilerplate that referenced this issue Feb 26, 2016
This just sets a flag to prevent the watch task from being restarted when the user closes the application. Probably not a perfect long-term solution, but enough to get the gears turning again!

Thanks goes out to @pkunze for finding the issue.
@jimbuck
Copy link

jimbuck commented Feb 26, 2016

I've submitted a PR with a simple fix for the auto-restart when closing. It just sets a flag when it should not restart the watch task. The only question left is what happens when you forcefully stop the initial node process (hit Ctrl + C twice). Does it clean up the child processes correctly?

@carlosperate
Copy link
Contributor

If I remember correctly "tree-kill was added exclusively to try to fix this issue, so if this PR offers a solution that. in addition, allows us to remove a dependency it sounds good to me.

@szwacz
Copy link
Owner

szwacz commented Feb 26, 2016

Yes tree-kill was added specially for this purpose.
Actuall removing tree-kill completely for me solved the issue. Can you try that as well?
Maybe newer version of node just works and no workaround is necessery.

@jimbuck
Copy link

jimbuck commented Feb 26, 2016

I will try it later today or tomorrow and definitely let you know!

@jimbuck
Copy link

jimbuck commented Feb 29, 2016

So after removing tree-kill it seems to be doing better, but still not that great. One thing I did try was removing the reference to the node_modules folder from line 19 and then re-added it as necessary (line 38). I need to keep testing it, but maybe the watch task was just having a hard time keeping track of all the dependencies.

@szwacz
Copy link
Owner

szwacz commented Feb 29, 2016

@JimmyBoh if you think big count of files in node_modules might be a factor then you can just comment the watch task for 'copy-watch' and see how it goes then.

@szwacz
Copy link
Owner

szwacz commented Mar 19, 2016

Watch task has been refactored. Now using gulp-plumber and gulp-watch without creating child processes. Hope this solves all issues with watch task. Please give it a try.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants