search

fzaninotto / screenshot-as-a-service

Website screenshot service powered by node.js and phantomjs
1.1k stars 243 forks source link

Do not start 2 new rasterizers on restart. #41

Closed nkouevda closed 11 years ago

nkouevda commented 11 years ago

restartService kills the rasterizer and starts a new one, but the killed rasterizer also calls startService on its own exit event. As a result, every time restartService is called, 2 new rasterizers are spawned for the 1 killed.

Since the automatic startService on exit should remain intact, the startService within restartService should be removed.

This is probably related to what @lahdekorpi and @HaNdTriX report in #32.

HaNdTriX commented 11 years ago

Thanks @nkouevda !

I just tested your pull request. Unfortunately it results in stopping the phantomjs process and not starting a new one.

I believe, that the rasterizer.on('exit', ...) event never get's called in this case.

Removing the second startService will result in the same problem.

nkouevda commented 11 years ago

@HaNdTriX How did you test this? I can't reproduce the behavior you've detailed.

nkouevda commented 11 years ago

@HaNdTriX I still haven't been able to reproduce what you said, but I've changed my fix anyway. Instead of removing startService from restartService, killService removes the exit handler before killing the rasterizer. This way, killService actually kills the rasterizer (in the past, it would've restarted itself), and in the case that it fails to do so, restartService will still start a new rasterizer.

HaNdTriX commented 11 years ago

@nkouevda thanks! Works for me!

fzaninotto commented 11 years ago

Thanks!

daviddumon commented 11 years ago

Hello,

It seems there are still some issues with the restartService. Here is my log :+1:

Request for http://www.mongohq.com/home - Rasterizing it Sending image in response phantomjs error: PhantomJS has crashed. Please read the crash reporting guide at https://github.com/ariya/phantomjs/wiki/Crash-Reporting and file a bug report at https://github.com/ariya/phantomjs/issues/new with the crash dump file attached: /tmp/1f5b12b4-4aaf-bdca-5429b03e-06acd95d.dmp

phantomjs failed; restarting [uncaughtException] [TypeError: Object # has no method 'startService']

Thank you for this very useful project !

David.