diff --git a/.travis.yml b/.travis.yml index bf99098..194b715 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,11 @@ -language: cpp -compiler: gcc +language: php +php: + - 5.5 + - 5.4 + - 5.3 +env: + - V8VER=3.22.22 + - V8VER=3.21.12 before_install: make -f Makefile.travis before_install install: make -f Makefile.travis install script: make -f Makefile.travis test diff --git a/Makefile.travis b/Makefile.travis index 190f17c..ef18367 100644 --- a/Makefile.travis +++ b/Makefile.travis @@ -1,6 +1,6 @@ # Configure and build scripts for travis CI system -V8VER=3.22.10 -CPPVER=4.6 +V8VER ?= 3.22.10 +CPPVER ?= 4.6 export CXX=g++-$(CPPVER) export LINK=g++-$(CPPVER) diff --git a/php_v8js_macros.h b/php_v8js_macros.h index 59b4f12..d0e8cf7 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -142,8 +142,18 @@ v8::Handle zval_to_v8js(zval *, v8::Isolate * TSRMLS_DC); /* Convert V8 value into zval */ int v8js_to_zval(v8::Handle, zval *, int, v8::Isolate * TSRMLS_DC); +struct php_v8js_accessor_ctx +{ + char *variable_name_string; + uint variable_name_string_len; + v8::Isolate *isolate; +}; + +void php_v8js_accessor_ctx_dtor(php_v8js_accessor_ctx * TSRMLS_DC); + /* Register accessors into passed object */ -void php_v8js_register_accessors(v8::Local, zval *, v8::Isolate * TSRMLS_DC); +void php_v8js_register_accessors(std::vector *accessor_list, v8::Local, zval *, v8::Isolate * TSRMLS_DC); + /* {{{ Context container */ struct php_v8js_ctx { @@ -161,6 +171,7 @@ struct php_v8js_ctx { std::vector modules_stack; std::vector modules_base; std::map template_cache; + std::vector accessor_list; #ifdef ZTS void ***zts_ctx; #endif @@ -180,6 +191,7 @@ struct php_v8js_timer_ctx long memory_limit; std::chrono::time_point time_point; php_v8js_ctx *v8js_ctx; + bool killed; }; /* {{{ Object container */ @@ -188,6 +200,7 @@ struct php_v8js_object { v8::Persistent v8obj; int flags; v8::Isolate *isolate; + HashTable *properties; }; /* }}} */ diff --git a/v8js.cc b/v8js.cc index 6350d6d..ec56e00 100644 --- a/v8js.cc +++ b/v8js.cc @@ -305,8 +305,16 @@ static HashTable *php_v8js_v8_get_properties(zval *object TSRMLS_DC) /* {{{ */ php_v8js_object *obj = (php_v8js_object *) zend_object_store_get_object(object TSRMLS_CC); HashTable *retval; - ALLOC_HASHTABLE(retval); - zend_hash_init(retval, 0, NULL, ZVAL_PTR_DTOR, 0); + if (obj->properties == NULL) { + if (GC_G(gc_active)) { + /* the garbage collector is running, don't create more zvals */ + return NULL; + } + ALLOC_HASHTABLE(obj->properties); + zend_hash_init(obj->properties, 0, NULL, ZVAL_PTR_DTOR, 0); + } else { + zend_hash_clean(obj->properties); + } v8::Isolate *isolate = obj->isolate; v8::Locker locker(isolate); @@ -316,8 +324,8 @@ static HashTable *php_v8js_v8_get_properties(zval *object TSRMLS_DC) /* {{{ */ v8::Context::Scope temp_scope(temp_context); v8::Local v8obj = v8::Local::New(isolate, obj->v8obj); - if (php_v8js_v8_get_properties_hash(v8obj, retval, obj->flags, isolate TSRMLS_CC) == SUCCESS) { - return retval; + if (php_v8js_v8_get_properties_hash(v8obj, obj->properties, obj->flags, isolate TSRMLS_CC) == SUCCESS) { + return obj->properties; } return NULL; @@ -326,7 +334,7 @@ static HashTable *php_v8js_v8_get_properties(zval *object TSRMLS_DC) /* {{{ */ static HashTable *php_v8js_v8_get_debug_info(zval *object, int *is_temp TSRMLS_DC) /* {{{ */ { - *is_temp = 1; + *is_temp = 0; return php_v8js_v8_get_properties(object TSRMLS_CC); } /* }}} */ @@ -460,6 +468,12 @@ static void php_v8js_v8_free_storage(void *object, zend_object_handle handle TSR { php_v8js_object *c = (php_v8js_object *) object; + if (c->properties) { + zend_hash_destroy(c->properties); + FREE_HASHTABLE(c->properties); + c->properties = NULL; + } + zend_object_std_dtor(&c->std TSRMLS_CC); c->v8obj.Reset(); @@ -495,6 +509,7 @@ void php_v8js_create_v8(zval *res, v8::Handle value, int flags, v8::I c->v8obj.Reset(isolate, value); c->flags = flags; c->isolate = isolate; + c->properties = NULL; } /* }}} */ @@ -512,6 +527,10 @@ static void php_v8js_free_storage(void *object TSRMLS_DC) /* {{{ */ zval_ptr_dtor(&c->pending_exception); } + if (c->module_loader) { + zval_ptr_dtor(&c->module_loader); + } + /* Delete PHP global object from JavaScript */ if (!c->context.IsEmpty()) { v8::Locker locker(c->isolate); @@ -535,6 +554,13 @@ static void php_v8js_free_storage(void *object TSRMLS_DC) /* {{{ */ } c->template_cache.~map(); + /* Clear contexts */ + for (std::vector::iterator it = c->accessor_list.begin(); + it != c->accessor_list.end(); ++it) { + php_v8js_accessor_ctx_dtor(*it TSRMLS_CC); + } + c->accessor_list.~vector(); + /* Clear global object, dispose context */ if (!c->context.IsEmpty()) { c->context.Reset(); @@ -581,6 +607,7 @@ static zend_object_value php_v8js_new(zend_class_entry *ce TSRMLS_DC) /* {{{ */ new(&c->modules_stack) std::vector(); new(&c->modules_base) std::vector(); new(&c->template_cache) std::map(); + new(&c->accessor_list) std::vector(); retval.handle = zend_objects_store_put(c, NULL, (zend_objects_free_object_storage_t) php_v8js_free_storage, NULL TSRMLS_CC); retval.handlers = &v8js_object_handlers; @@ -799,7 +826,7 @@ static PHP_METHOD(V8Js, __construct) /* Register Get accessor for passed variables */ if (vars_arr && zend_hash_num_elements(Z_ARRVAL_P(vars_arr)) > 0) { - php_v8js_register_accessors(php_obj_t->InstanceTemplate(), vars_arr, isolate TSRMLS_CC); + php_v8js_register_accessors(&c->accessor_list, php_obj_t, vars_arr, isolate TSRMLS_CC); } /* Set name for the PHP JS object */ @@ -870,6 +897,7 @@ static void php_v8js_timer_push(long time_limit, long memory_limit, php_v8js_ctx timer_ctx->memory_limit = memory_limit; timer_ctx->time_point = from + duration; timer_ctx->v8js_ctx = c; + timer_ctx->killed = false; V8JSG(timer_stack).push(timer_ctx); V8JSG(timer_mutex).unlock(); @@ -880,12 +908,11 @@ static void php_v8js_timer_pop(TSRMLS_D) V8JSG(timer_mutex).lock(); if (V8JSG(timer_stack).size()) { - // Free the timer context memory php_v8js_timer_ctx *timer_ctx = V8JSG(timer_stack).top(); - efree(timer_ctx); - // Remove the timer context from the stack V8JSG(timer_stack).pop(); + // Free the timer context memory + efree(timer_ctx); } V8JSG(timer_mutex).unlock(); @@ -895,9 +922,7 @@ static void php_v8js_terminate_execution(php_v8js_ctx *c TSRMLS_DC) { // Forcefully terminate the current thread of V8 execution in the isolate v8::V8::TerminateExecution(c->isolate); - - // Remove this timer from the stack - php_v8js_timer_pop(TSRMLS_C); + // This timer will be removed from stack by the parent thread. } static void php_v8js_timer_thread(TSRMLS_D) @@ -914,7 +939,9 @@ static void php_v8js_timer_thread(TSRMLS_D) // Get memory usage statistics for the isolate c->isolate->GetHeapStatistics(&hs); - if (timer_ctx->time_limit > 0 && now > timer_ctx->time_point) { + if (timer_ctx->time_limit > 0 && now > timer_ctx->time_point && + !timer_ctx->killed) { + timer_ctx->killed = true; php_v8js_terminate_execution(c TSRMLS_CC); V8JSG(timer_mutex).lock(); @@ -922,7 +949,9 @@ static void php_v8js_timer_thread(TSRMLS_D) V8JSG(timer_mutex).unlock(); } - if (timer_ctx->memory_limit > 0 && hs.used_heap_size() > timer_ctx->memory_limit) { + if (timer_ctx->memory_limit > 0 && hs.used_heap_size() > timer_ctx->memory_limit && + !timer_ctx->killed) { + timer_ctx->killed = true; php_v8js_terminate_execution(c TSRMLS_CC); V8JSG(timer_mutex).lock(); @@ -1001,7 +1030,7 @@ static PHP_METHOD(V8Js, executeString) v8::Local result = script->Run(); c->in_execution--; - if (time_limit > 0) { + if (time_limit > 0 || memory_limit > 0) { php_v8js_timer_pop(TSRMLS_C); } @@ -1670,6 +1699,16 @@ static PHP_MINFO_FUNCTION(v8js) */ static PHP_GINIT_FUNCTION(v8js) { + /* + If ZTS is disabled, the v8js_globals instance is declared right + in the BSS and hence automatically initialized by C++ compiler. + Most of the variables are just zeroed. + + If ZTS is enabled however, v8js_globals just points to a freshly + allocated, uninitialized piece of memory, hence we need to + initialize all fields on our own. Likewise on shutdown we have to + run the destructors manually. + */ #ifdef ZTS v8js_globals->extensions = NULL; v8js_globals->v8_initialized = 0; diff --git a/v8js_convert.cc b/v8js_convert.cc index c59e820..7a6afe0 100644 --- a/v8js_convert.cc +++ b/v8js_convert.cc @@ -29,6 +29,8 @@ extern "C" { #include #include +static void php_v8js_weak_object_callback(const v8::WeakCallbackData &data); + /* Callback for PHP methods and functions */ static void php_v8js_call_php_func(zval *value, zend_class_entry *ce, zend_function *method_ptr, v8::Isolate *isolate, const v8::FunctionCallbackInfo& info TSRMLS_DC) /* {{{ */ { @@ -178,6 +180,7 @@ static void php_v8js_construct_callback(const v8::FunctionCallbackInfo newobj = info.This(); v8::Local php_object; + zval *value; if (!info.IsConstructCall()) { return; @@ -190,6 +193,10 @@ static void php_v8js_construct_callback(const v8::FunctionCallbackInfoIsExternal()) { // Object created by v8js in php_v8js_hash_to_jsobj, PHP object passed as v8::External. php_object = v8::Local::Cast(info[0]); + value = reinterpret_cast(php_object->Value()); + // Increase the reference count of this value because we're storing it internally for use later + // See https://github.com/preillyme/v8js/issues/6 + Z_ADDREF_P(value); } else { // Object created from JavaScript context. Need to create PHP object first. V8JS_TSRMLS_FETCH(); @@ -202,7 +209,6 @@ static void php_v8js_construct_callback(const v8::FunctionCallbackInfoSetAlignedPointerInInternalField(0, ext_tmpl->Value()); newobj->SetHiddenValue(V8JS_SYM(PHPJS_OBJECT_KEY), php_object); + + // Since we got to decrease the reference count again, in case v8 garbage collector + // decides to dispose the JS object, we add a weak persistent handle and register + // a callback function that removes the reference. + v8::Persistent persist_newobj(isolate, newobj); + persist_newobj.SetWeak(value, php_v8js_weak_object_callback); + + // Just tell v8 that we're allocating some external memory + // (for the moment we just always tell 1k instead of trying to find out actual values) + v8::V8::AdjustAmountOfExternalAllocatedMemory(1024); } /* }}} */ @@ -499,8 +515,7 @@ static inline v8::Local php_v8js_named_property_callback(v8::Local cb; V8JS_TSRMLS_FETCH(); - zend_class_entry *scope = NULL; /* XXX? */ - zend_class_entry *ce; + zend_class_entry *scope, *ce; zend_function *method_ptr = NULL; zval *php_value; @@ -508,7 +523,7 @@ static inline v8::Local php_v8js_named_property_callback(v8::Local tmpl = v8::Local::New (isolate, *reinterpret_cast(self->GetAlignedPointerFromInternalField(0))); - ce = Z_OBJCE_P(object); + ce = scope = Z_OBJCE_P(object); /* First, check the (case-insensitive) method table */ php_strtolower(lower, name_len); @@ -583,8 +598,15 @@ static inline v8::Local php_v8js_named_property_callback(v8::Local php_v8js_hash_to_jsobj(zval *value, v8::Isolate *is v8::Handle external = v8::External::New(value); newobj = new_tpl->GetFunction()->NewInstance(1, &external); - // Increase the reference count of this value because we're storing it internally for use later - // See https://github.com/preillyme/v8js/issues/6 - Z_ADDREF_P(value); - - // Since we got to decrease the reference count again, in case v8 garbage collector - // decides to dispose the JS object, we add a weak persistent handle and register - // a callback function that removes the reference. - v8::Persistent persist_newobj(isolate, newobj); - persist_newobj.SetWeak(value, php_v8js_weak_object_callback); - - // Just tell v8 that we're allocating some external memory - // (for the moment we just always tell 1k instead of trying to find out actual values) - v8::V8::AdjustAmountOfExternalAllocatedMemory(1024); - if (ce == zend_ce_closure) { // free uncached function template when object is freed v8::Persistent persist_newobj2(isolate, newobj); diff --git a/v8js_methods.cc b/v8js_methods.cc index 0c0d5f2..133e4f9 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -234,6 +234,7 @@ V8JS_METHOD(require) for (std::vector::iterator it = c->modules_stack.begin(); it != c->modules_stack.end(); ++it) { if (!strcmp(*it, normalised_module_id)) { + efree(normalised_module_id); efree(normalised_path); info.GetReturnValue().Set(v8::ThrowException(V8JS_SYM("Module cyclic dependency"))); @@ -243,6 +244,7 @@ V8JS_METHOD(require) // If we have already loaded and cached this module then use it if (V8JSG(modules_loaded).count(normalised_module_id) > 0) { + efree(normalised_module_id); efree(normalised_path); info.GetReturnValue().Set(V8JSG(modules_loaded)[normalised_module_id]); @@ -251,22 +253,26 @@ V8JS_METHOD(require) // Callback to PHP to load the module code - zval module_code; + zval *module_code; zval *normalised_path_zend; MAKE_STD_ZVAL(normalised_path_zend); ZVAL_STRING(normalised_path_zend, normalised_module_id, 1); - zval* params[] = { normalised_path_zend }; - if (FAILURE == call_user_function(EG(function_table), NULL, c->module_loader, &module_code, 1, params TSRMLS_CC)) { + zval **params[1] = {&normalised_path_zend}; + if (FAILURE == call_user_function_ex(EG(function_table), NULL, c->module_loader, &module_code, 1, params, 0, NULL TSRMLS_CC)) { + zval_ptr_dtor(&normalised_path_zend); + efree(normalised_module_id); efree(normalised_path); info.GetReturnValue().Set(v8::ThrowException(V8JS_SYM("Module loader callback failed"))); return; } + zval_ptr_dtor(&normalised_path_zend); // Check if an exception was thrown if (EG(exception)) { + efree(normalised_module_id); efree(normalised_path); // Clear the PHP exception and throw it in V8 instead @@ -276,12 +282,14 @@ V8JS_METHOD(require) } // Convert the return value to string - if (Z_TYPE(module_code) != IS_STRING) { - convert_to_string(&module_code); + if (Z_TYPE_P(module_code) != IS_STRING) { + convert_to_string(module_code); } // Check that some code has been returned - if (!strlen(Z_STRVAL(module_code))) { + if (Z_STRLEN_P(module_code)==0) { + zval_ptr_dtor(&module_code); + efree(normalised_module_id); efree(normalised_path); info.GetReturnValue().Set(v8::ThrowException(V8JS_SYM("Module loader callback did not return code"))); @@ -321,13 +329,15 @@ V8JS_METHOD(require) // Set script identifier v8::Local sname = V8JS_SYM("require"); - v8::Local source = V8JS_STR(Z_STRVAL(module_code)); + v8::Local source = V8JS_STRL(Z_STRVAL_P(module_code), Z_STRLEN_P(module_code)); + zval_ptr_dtor(&module_code); // Create and compile script v8::Local script = v8::Script::New(source, sname); // The script will be empty if there are compile errors if (script.IsEmpty()) { + efree(normalised_module_id); efree(normalised_path); info.GetReturnValue().Set(v8::ThrowException(V8JS_SYM("Module script compile failed"))); return; @@ -345,6 +355,7 @@ V8JS_METHOD(require) c->modules_stack.pop_back(); c->modules_base.pop_back(); + efree(normalised_module_id); efree(normalised_path); // Script possibly terminated, return immediately diff --git a/v8js_variables.cc b/v8js_variables.cc index fa81b64..3e6ddf3 100644 --- a/v8js_variables.cc +++ b/v8js_variables.cc @@ -25,13 +25,6 @@ extern "C" { #include #include -struct php_v8js_accessor_ctx -{ - char *variable_name_string; - uint variable_name_string_len; - v8::Isolate *isolate; -}; - static void php_v8js_fetch_php_variable(v8::Local name, const v8::PropertyCallbackInfo& info) /* {{{ */ { v8::Handle data = v8::Handle::Cast(info.Data()); @@ -50,12 +43,20 @@ static void php_v8js_fetch_php_variable(v8::Local name, const v8::Pr } /* }}} */ -void php_v8js_register_accessors(v8::Local php_obj, zval *array, v8::Isolate *isolate TSRMLS_DC) /* {{{ */ +void php_v8js_accessor_ctx_dtor(php_v8js_accessor_ctx *ctx TSRMLS_DC) /* {{{ */ +{ + efree(ctx->variable_name_string); + efree(ctx); +} +/* }}} */ + +void php_v8js_register_accessors(std::vector *accessor_list, v8::Local php_obj_t, zval *array, v8::Isolate *isolate TSRMLS_DC) /* {{{ */ { char *property_name; uint property_name_len; ulong index; zval **item; + v8::Local php_obj = php_obj_t->InstanceTemplate(); for (zend_hash_internal_pointer_reset(Z_ARRVAL_P(array)); zend_hash_get_current_data(Z_ARRVAL_P(array), (void **) &item) != FAILURE; @@ -85,7 +86,10 @@ void php_v8js_register_accessors(v8::Local php_obj, zval *ar ctx->isolate = isolate; /* Set the variable fetch callback for given symbol on named property */ - php_obj->SetAccessor(V8JS_STRL(property_name, property_name_len - 1), php_v8js_fetch_php_variable, NULL, v8::External::New(ctx), v8::PROHIBITS_OVERWRITING, v8::ReadOnly); + php_obj->SetAccessor(V8JS_STRL(property_name, property_name_len - 1), php_v8js_fetch_php_variable, NULL, v8::External::New(ctx), v8::PROHIBITS_OVERWRITING, v8::ReadOnly, v8::AccessorSignature::New(php_obj_t)); + + /* record the context so we can free it later */ + accessor_list->push_back(ctx); } } /* }}} */