search

peerigon / phridge

A bridge between node and PhantomJS
The Unlicense
519 stars 50 forks source link

Orphaned phantomjs processes #33

Closed SystemParadox closed 8 years ago

SystemParadox commented 9 years ago

Hi. This might just be a documentation issue, but I am not clear how to correctly cleanup the phantomJS processes when node exits either due to a SIGTERM or an uncaught exception.

At the moment we are getting a collection of orphaned phantomJS processes which grows whenever we restart node.

The documentation specifically states that it is not safe to call phantom.disposeAll() from process.on('exit') because it is async. But when an exception occurs, or when the node process is being terminated (i.e. by a supervisor), the only way to reliably cleanup is to use process.on('exit') and do it synchronously.

How should I best deal with this? Maybe we need a phridge.killAll()? Although, shouldn't it be phridge's responsibility to ensure that any child processes it created are definitely cleaned up when node exits?

Thanks.

jhnns commented 9 years ago

This is closely related to #34 and #35, I guess. Let me sort that out:

SIGTERM

It should be possible to exit cleanly with disposeAll() when a SIGTERM is received. I've tested it on my linux VM and couldn't find a PhantomJs orphan afterwards. My linux VM is:

vagrant@dev:/vagrant/temp/phridge-epipe$ lsb_release -irc
Distributor ID: Ubuntu
Release:    12.04
Codename:   precise

Uncaught exception

You should probably listen on process.on('uncaughtException'), dispose all phridge instances and then exit with process.exit(1).

I guess it heavily depends on the OS you're using. Some OS will automatically kill child processes, some won't. If your OS does not kill child processes, you'll responsible to shut them down. From phridge's perspective, it is not possible to shutdown child processes synchronously because we're just using the stdin stream which is asynchronous in node.

jhnns commented 8 years ago

Could you checkout the current master? I'm explicitly killing the childProcess now if something goes wrong. This should leave no orphans....

jhnns commented 8 years ago

Is this still an issue with v1.2.0?

SystemParadox commented 8 years ago

I've been running this for a while and it seems to be ok :)

Thanks very much