phpv8 / v8js

V8 Javascript Engine for PHP — This PHP extension embeds the Google V8 Javascript Engine
http://pecl.php.net/package/v8js
MIT License
1.84k stars 200 forks source link

Time limit applied via setTimeLimit only enforced after calls to sleep complete #203

Closed mikemherron closed 8 years ago

mikemherron commented 8 years ago

This may be intended behaviour, but if sleep() is called with a duration greater than the script time limit (set via setTimeLimit), the V8JsTimeLimitException exception is thrown, but only once the sleep period has finished. The below script demonstrates this issue:

<?php

$v8 = new V8Js();
$v8->setTimeLimit( 2000 );

$start = time();
try {
    $v8->executeString( "sleep(10);");    
}
catch( V8JsTimeLimitException $exception )
{
    $duration = time() - $start;
    printf( "\nV8JsTimeLimitException thrown after %d seconds\n", $duration );
}

This will print out:

V8JsTimeLimitException thrown after 10 seconds

Given this, is there a way to stop the global sleep function from becoming available to scripts?

stesie commented 8 years ago

I don't think it is intended behaviour - and I consider it at least inconsistent behaviour wrt PHP core. And I already slightly changed V8Js' original behaviour by adding the setTimeLimit method behaving much like PHP's set_time_limit which e.g. resets the counter un succeeding calls

PHP's sleep however doesn't count towards the set_time_limit limit, ... hence I'd argue that V8Js::setTimeLimit should neither

That is the above script should not be terminated ... however still block for 10 seconds.

However it's not currently possible to not export the sleep function to V8. As a work-around for the moment you may want to just $v8->executeString("delete this.sleep"); right after instanciating V8Js -- but that only solves the problem for the primary context. If you use the common.js module loader the sleep function will be available again there.

I think neither a php.ini setting nor a setter on the V8Js object to hide the sleep function are neat solutions, since e.g. the exit function pretty easily can get as problematic as the other. Leading to "all of them should be hideable" ... which brings me back to issue #72 where the rationale is to switch to V8Js' normal object mapping for exporting the root and PHP object. There I'd like to have a trait with all the common function which you can import with method visibility of e.g. sleep and exit changed to private.

So is the work-around outlined above feasible to you?

mikemherron commented 8 years ago

Thanks for the detailed reply. #72 is an interesting discussion, though to be honest much of it goes beyond my understanding of the V8 internals. I had made some local modifications to provide a V8Js constructor argument that disables the built in methods (sleep, exit, print, var_dump), but I agree that it's not a nice solution really.

The $v8->executeString("delete this.sleep"); work around is totally acceptable for my use case, thanks.