fzaninotto / screenshot-as-a-service

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

Phantomjs Processes Won't Stop #32

Open lahdekorpi opened 11 years ago

lahdekorpi commented 11 years ago

My test case:

And the result is oprhaned phantomjs processes consuming memory: Kuvankaappaus 2013-4-8 kello 16 21 34

Crash:

Sending image in response
[uncaughtException] { [Error: ENOENT, no such file or directory '/tmp/screenshot_a0759a220b50b287388af42c0149a3fe.png']
  errno: 34,
  code: 'ENOENT',
  path: '/tmp/screenshot_a0759a220b50b287388af42c0149a3fe.png',
  syscall: 'unlink' }
Stopping Phantomjs internal server

fs.js:760
  return binding.unlink(pathModule._makeLong(path));
                 ^
Error: ENOENT, no such file or directory '/tmp/screenshot_b49924b800f523f831a2c84f4ebcd53b.png'
    at Object.fs.unlinkSync (fs.js:760:18)
    at FileCleanerService.removeFile (/opt/screenshot-service/screenshot-as-a-service/lib/fileCleanerService.js:41:6)
    at FileCleanerService.removeAllFiles (/opt/screenshot-service/screenshot-as-a-service/lib/fileCleanerService.js:46:10)
    at process.<anonymous> (/opt/screenshot-service/screenshot-as-a-service/lib/fileCleanerService.js:21:10)
    at process.EventEmitter.emit (events.js:117:20)
    at process.exit (node.js:707:17)
    at process.app.configure.app.use.express.errorHandler.dumpExceptions (/opt/screenshot-service/screenshot-as-a-service/app.js:11:11)
lahdekorpi commented 11 years ago

Maybe this bug should be cut into two;

I'm guessing some request already had the same url at the same time, so one of them removed it before the other. So just wrapping the unlink function into try-catch should work?

curana commented 11 years ago

This commit fixes the crash but the response remains empty. I think the caching mechanism needs some improvement.

Though I could not reproduce the big amount of phantomjs processes.

lahdekorpi commented 11 years ago

Any ideas how to improve?

As for the processes, I got them with tens of thousands of requests in a couple of minutes...

curana commented 11 years ago

What version of phantomJS are you running?

About the crash: The filename is based on a md5 hash of the options. If you switch on caching the time to elapse until you have this error should increase. If you do not use caching at all you can just add e.g. the timestamp to the filename creation.

For me the main problem seems to be that this script tries to read the screenshot, fails but does not have a fallback. I dont know if the following process would improve the situation:

I did not completely think about a "final solution". Maybe there are other drawbacks to handle in this case.

lahdekorpi commented 11 years ago
# phantomjs -v
1.9.0
# node -v
v0.10.3

Yeah, I might just change the naming and see how it goes, thanks, but that doesn't solve problems that might be caused by disk errors, etc. It's common to use NFS in this kind of situations and I'm assuming a short network hiccup would crash the whole service.

curana commented 11 years ago

If you apply the patch the service will not crash but return an empty response. I dont know if you could live with that.

You could implement a fallback if the file is not found to retry taking a screenshot, maybe with exclusive naming so nobody can delete this one special file which was created for this one request.

If I find a great solution I will let yo know.

HaNdTriX commented 11 years ago

I am having a similar Issue.

The service works for about 3000 Screenshots. But when it tries to restart itself. The this.rasterizer.kill(); does not work properly.

It fails to stop the server somehow.

Phantomjs process is sleeping. Restarting.
Stopping Phantomjs internal server
Phantomjs internal server listening on port 3001
phantomjs failed; restarting

This will go on unit I have up to 40 phantomjs servers running.

I used the latest version of screenshot-as-a-service and:

node --version
v0.11.3-pre

phantomjs --version
1.9.0

Does anyone know a Solution?

ebuildy commented 10 years ago

I have same issue, phantomjs process are staying as phantom process ^^

Maybe a kind of cron task or with supervisord to kill these process ?