Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Orphaned phantomjs processes #33

Closed
SystemParadox opened this issue May 14, 2015 · 4 comments
Closed

Orphaned phantomjs processes #33

SystemParadox opened this issue May 14, 2015 · 4 comments

Comments

@SystemParadox
Copy link
Contributor

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
Copy link
Member

jhnns commented May 15, 2015

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
Copy link
Member

jhnns commented Sep 25, 2015

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

@jhnns
Copy link
Member

jhnns commented Oct 5, 2015

Is this still an issue with v1.2.0?

@jhnns jhnns closed this as completed Jan 31, 2016
@SystemParadox
Copy link
Contributor Author

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

Thanks very much

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

2 participants