Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory / time limit #7

Closed
andrewtch opened this issue Mar 26, 2013 · 27 comments
Closed

Memory / time limit #7

andrewtch opened this issue Mar 26, 2013 · 27 comments
Assignees

Comments

@andrewtch
Copy link
Contributor

Hi,

I'm planning to use your library for triggers / data processing, and it would be nice to have time limiting (and probably memory limiting, which is not that important).

This can be passed to executeString as an array of options after $flags parameter. Also, I can try to pullrequest if you can point where to change, but my C knowledge is not that good )

@satoshi75nakamoto
Copy link
Contributor

@andrewtch — I'll take a stab at it later this week. But, feel free to try as well.

@andrewtch
Copy link
Contributor Author

Digging in, I could not find any time limits for v8 engine itself, so maybe it's not possible at all.

@beest
Copy link
Collaborator

beest commented Mar 31, 2013

@andrewtch you can use PHP's set_time_limit to achieve the same result. It's not the most elegant solution but it does work.

If you need to do any post-timeout clean-up then take a look at register_shutdown_function, which does get triggered after a timeout.

Here's an example:

function shutdown()
{
    print('Shutting down' . PHP_EOL);
}

register_shutdown_function('shutdown');
set_time_limit(1);

$code = <<<EOT

var text = "testing";
for (var i = 0; i < 10000000; ++i) {
    var encoded = encodeURI(text);
}

EOT;

$v8Js = new V8Js();
$v8Js->executeString($code);

@andrewtch
Copy link
Contributor Author

I'm aware of that. There's no goal to end PHP script after some timeout. The goal is to terminate v8 script and continue PHP script.

@beest
Copy link
Collaborator

beest commented Mar 31, 2013

That's exactly what it does. Give it a try.

@andrewtch
Copy link
Contributor Author

@beest , no, it isn't.

Assume the following:

foreach ($objects as $object) {
    foreach ($objects->getCallbacks() as $callback) {
        try {
            $v8->executeString($callback->getCode());
        } catch (V8TimeoutException $e) {
              $this->log('callback timeout, callback id: '.$callback->getId());
        }
    }
}

and this is what I actually want.

I do not want to stop the script. I must have all objects and callbacks processed. With your approach I would need to process rest of the objects and callbacks in shutdown function, and what if another script terminates there, do some nesting woodoo with shutdown function?

@beest
Copy link
Collaborator

beest commented Apr 1, 2013

There's nothing available natively in V8 or PHP. You'll need to look at thread synchronization to achieve this.

I'm sure I could put something together to do this, and sounds like @preillyme is going to take a look too. It would indeed be a useful feature.

@andrewtch
Copy link
Contributor Author

Yep, as I stated above there are no limits in v8 itself. This week I'll try to play with gearman and see how it works with v8, and I'll be much appreciated if you can look on this issue from PHP / V8 side.

@MarkOfSine
Copy link

Half-a-tangent...
We need similar functionality (i.e Executing a JS script on a backend server) and have looked at various options including spidermonkey and v8js. The challenge we always run into is the timeout issue. Our scripts will be written by external clients and as such we will not be able to vet them to ensure they are well written and do not consume excessive resources or take long to execute.

With this in mind, we are thinking of the following alternative solution:
Implement a NodeJs server using DNode RPC (https://github.com/substack/dnode).
The JS code is then called via a socket and timeouts can be implemented on both the Php & JS server side.

Js trigger execute

It is obviously more complex to implement but based on the timeout issue inherent with the built-in Php js engines I am not sure how else this could be effectively done.

So, question. Is this overkill, does anyone have an opinion on whether it would work and finally is there perhaps something we have missed with the built-in option that would enable to not go this alternate route?

@andrewtch
Copy link
Contributor Author

Please take a look on this implementation: http://www.mail-archive.com/v8-users@googlegroups.com/msg04249.html

@beest
Copy link
Collaborator

beest commented Apr 1, 2013

I've got an implementation of timeouts working in my fork: https://github.com/beest/v8js/tree/timeout

It throws a V8JsTimeoutException when a script exceeds a timeout passed in via executeString.

@andrewtch
Copy link
Contributor Author

maybe it would be better instead of using pthread, which is a big bottleneck in PHP, to use v8::internal::Thread as proposed here: http://pastebin.com/M801zbDB . This would go to native v8 threads instead of messing with phtread library.

@beest
Copy link
Collaborator

beest commented Apr 2, 2013

The v8::internal::Thread class is not in the standard version of the V8 library. The only place I can see it is here: http://blog.peschla.net/doxygen/v8_chromium_r157275/classv8_1_1internal_1_1_thread.html which seems to be a specific implementation of V8 embedded in the Chromium browser. If you have any more information then I'd happily take a look.

The use of pthreads shouldn't inherently be a problem. Note how the sample code you provided links to the pthread library anyway "-lpthread".

However, support for Windows is only provided via a pthreads port. When this version is completed I will look at how to achieve Windows support.

@beest
Copy link
Collaborator

beest commented Apr 2, 2013

I take that back - I can see the Thread class defined in the V8 code base. I'll dig a little deeper and see what I can do with it.

Note that this code also uses pthreads throughout for Linux and OSX. What are the PHP bottlenecks that you're aware of?

@andrewtch
Copy link
Contributor Author

At old days when I tried to hack PHP code (~5.1) the resources were the problem. For example, if you pass mysql_connect or fopen result to thread (assuming this is a shared connection / file handle), concurrent usage of this resource can/will segfault.

I suppose that the best practice would be to keep the code as is if there's no timeout, and do some woodoo if timeout is requested.

@andrewtch
Copy link
Contributor Author

Btw, the example above http://pastebin.com/M801zbDB also protects from infinite loops.

@beest
Copy link
Collaborator

beest commented Apr 15, 2013

FYI I've implemented both time and memory limiting in my fork at https://github.com/beest/v8js. It protects from any long-running or memory-intensive code, whether infinite loop or otherwise.

@andrewtch the example you provided was useful and this version works in a very similar way.

@preillyme take a look at the API additions and see if you'd be interested to include this functionality in the official extension.

@MarkOfSine this will give you exactly the functionality that you need.

@andrewtch
Copy link
Contributor Author

@beest, which branch?

@beest
Copy link
Collaborator

beest commented Apr 16, 2013

@andrewtch master

@andrewtch
Copy link
Contributor Author

Looks good :)

@andrewtch
Copy link
Contributor Author

@preillyme , ping!

@satoshi75nakamoto
Copy link
Contributor

@beest — Can you send a PR.

@satoshi75nakamoto
Copy link
Contributor

@beest — ping, can you submit a pull request with your changes.

@beest
Copy link
Collaborator

beest commented Apr 28, 2013

I'll get this done this week. Couple of tweaks required. Sorry for the delay!

@satoshi75nakamoto
Copy link
Contributor

@beest — any luck?

@beest
Copy link
Collaborator

beest commented May 9, 2013

Finally got this done. Hope it works out for you, please do give it a try and provide some feedback.

@satoshi75nakamoto
Copy link
Contributor

@beest — Okay I'll take a look. Thanks for all of your hard work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants