search

mdlawson / piping

Keep your code piping hot! Live code reloading without additional binaries
MIT License
219 stars 13 forks source link

Worker might die before killed #12

Closed mntnoe closed 8 years ago

mntnoe commented 8 years ago

See #10.

This fix adds a no-op callback to the event queue via setTimeout to keep the worker alive so any exit handlers (process.on('SIGTERM', ...)) can be called (see nodejs/node#4852). Note that, depending on how the scheduler works, the worker may still exit too early.

This can be the case with hapi, the fix for which I have reverted in this PR. If anybody experiences problems with for instance hapi not restarting correctly, we should put more energy in securing that exit handlers are called.

mdlawson commented 8 years ago

Merged and released as 0.3.1. Thanks, and sorry for the delay!

strawbrary commented 8 years ago

8e3e3d6d3938abb367df3400a31f656496b283ef is breaking the reloading for me. I see [piping] File index.js has changed, reloading. but the changes aren't applied to the running process. Downgrading to 0.3.0 fixes the issue. I'm on OS X 10.11, tested with Node v5 and v6.

mntnoe commented 8 years ago

@strawbrary does it help if you increase the timeout added in e02a019? If so that means we should find a way to ensure that the worker does not exit too early. I currently don't have time to look into it, though.

strawbrary commented 8 years ago

@mntnoe Didn't have any luck with that unfortunately. I tried values between 10 and 10000 ms.

However, changing worker.kill() to process.kill(worker.process.pid, 'SIGTERM') does fix the issue for me regardless of the timeout.

mdlawson commented 8 years ago

This definitely breaks reloading on all platforms for me, and the fix suggested by strawbrary didn't seem to work at least when I tested, so I've reverted this and published a new version which is the same as 0.3.0. Let me know if you have a chance to look into this again.

Regardless, If I get some time I'll probably do a full rewrite of this package and hopefully sort out things like this then.