0
0
mirror of https://github.com/phpv8/v8js.git synced 2025-01-03 19:11:53 +00:00

Don't use zend_string for registerExtension- It may be freed

... before module shutdown
(In NTS builds of PHP 7 (e.g. 7.0.9) only)

Fixes https://github.com/phpv8/v8js/issues/247

This no longer returns getExtensions() in the order in which they were
registered.
This is slightly wasteful of memory - A custom hash function and
equality for `const char*` might be possible.
This commit is contained in:
Tyson Andre 2016-08-04 10:42:45 -07:00 committed by Tyson Andre
parent 4161008417
commit a612ce9bdc
4 changed files with 52 additions and 65 deletions

View File

@ -27,6 +27,7 @@
#include <list>
#include <vector>
#include <mutex>
#include <unordered_map>
#include <cmath>
#ifndef isnan
@ -77,7 +78,6 @@ extern "C" {
#define ZEND_SLEEP_FUNC_NAME "__sleep"
#define ZEND_SET_STATE_FUNC_NAME "__set_state"
/* Convert zval into V8 value */
v8::Handle<v8::Value> zval_to_v8js(zval *, v8::Isolate * TSRMLS_DC);
@ -97,8 +97,10 @@ void v8js_register_accessors(std::vector<v8js_accessor_ctx*> *accessor_list, v8:
/* Forward declarations */
struct v8js_jsext;
struct v8js_timer_ctx;
/* Module globals */
ZEND_BEGIN_MODULE_GLOBALS(v8js)
// Thread-local cache whether V8 has been initialized so far
@ -145,7 +147,7 @@ struct _v8js_process_globals {
std::mutex lock;
#endif
HashTable *extensions;
std::unordered_map<std::string, v8js_jsext*> extensions;
/* V8 command line flags */
char *v8_flags;

View File

@ -163,11 +163,10 @@ static PHP_MSHUTDOWN_FUNCTION(v8js)
v8js_process_globals.v8_flags = NULL;
}
if (v8js_process_globals.extensions) {
zend_hash_destroy(v8js_process_globals.extensions);
free(v8js_process_globals.extensions);
v8js_process_globals.extensions = NULL;
for (auto& it: v8js_process_globals.extensions) {
v8js_jsext_free_storage(it.second);
}
v8js_process_globals.extensions.clear();
return SUCCESS;
}

View File

@ -18,6 +18,8 @@
#include <functional>
#include <algorithm>
#include <string>
#include <unordered_map>
#include "php_v8js_macros.h"
#include "v8js_v8.h"
@ -62,11 +64,10 @@ int le_v8js_script;
/* {{{ Extension container */
struct v8js_jsext {
zend_bool auto_enable;
HashTable *deps_ht;
const char **deps;
int deps_count;
zend_string *name;
zend_string *source;
const char *name;
const char *source;
v8::Extension *extension;
};
/* }}} */
@ -260,29 +261,19 @@ static void v8js_free_ext_strarr(const char **arr, int count) /* {{{ */
}
/* }}} */
static void v8js_jsext_free_storage(v8js_jsext *jsext) /* {{{ */
void v8js_jsext_free_storage(v8js_jsext *jsext) /* {{{ */
{
if (jsext->deps_ht) {
zend_hash_destroy(jsext->deps_ht);
free(jsext->deps_ht);
}
if (jsext->deps) {
v8js_free_ext_strarr(jsext->deps, jsext->deps_count);
}
delete jsext->extension;
zend_string_release(jsext->name);
zend_string_release(jsext->source);
free((char*) jsext->name);
free((char*) jsext->source);
free(jsext);
}
/* }}} */
static void v8js_jsext_dtor(zval *zv) /* {{{ */
{
v8js_jsext_free_storage(reinterpret_cast<v8js_jsext *>(Z_PTR_P(zv)));
}
/* }}} */
static int v8js_create_ext_strarr(const char ***retval, int count, HashTable *ht) /* {{{ */
{
const char **exts = NULL;
@ -897,13 +888,6 @@ static void v8js_persistent_zval_ctor(zval *p) /* {{{ */
}
/* }}} */
static void v8js_persistent_zval_dtor(zval *p) /* {{{ */
{
assert(Z_TYPE_P(p) == IS_STRING);
free(Z_STR_P(p));
}
/* }}} */
static void v8js_script_free(v8js_script *res)
{
efree(res->name);
@ -925,18 +909,24 @@ static void v8js_script_dtor(zend_resource *rsrc) /* {{{ */
}
/* }}} */
static char* zend_string_to_new_cstring(zend_string *string) { /* {{{ */
char* s = (char*) malloc(ZSTR_LEN(string) + 1);
memcpy(s, ZSTR_VAL(string), ZSTR_LEN(string));
s[ZSTR_LEN(string)] = '\0';
return s;
}
/* }}} */
static int v8js_register_extension(zend_string *name, zend_string *source, zval *deps_arr, zend_bool auto_enable TSRMLS_DC) /* {{{ */
{
v8js_jsext *jsext = NULL;
std::string key(ZSTR_VAL(name), ZSTR_LEN(name));
#ifdef ZTS
v8js_process_globals.lock.lock();
#endif
if (!v8js_process_globals.extensions) {
v8js_process_globals.extensions = (HashTable *) malloc(sizeof(HashTable));
zend_hash_init(v8js_process_globals.extensions, 1, NULL, v8js_jsext_dtor, 1);
} else if (zend_hash_exists(v8js_process_globals.extensions, name)) {
if (v8js_process_globals.extensions.find(key) != v8js_process_globals.extensions.end()) {
#ifdef ZTS
v8js_process_globals.lock.unlock();
#endif
@ -959,24 +949,15 @@ static int v8js_register_extension(zend_string *name, zend_string *source, zval
}
jsext->auto_enable = auto_enable;
jsext->name = zend_string_dup(name, 1);
jsext->source = zend_string_dup(source, 1);
// TODO helper method
// Allocate character pointers, which need to outlive the allocated v8::Extension
jsext->name = zend_string_to_new_cstring(name);
jsext->source = zend_string_to_new_cstring(source);
if (jsext->deps) {
jsext->deps_ht = (HashTable *) malloc(sizeof(HashTable));
zend_hash_init(jsext->deps_ht, jsext->deps_count, NULL, v8js_persistent_zval_dtor, 1);
zend_hash_copy(jsext->deps_ht, Z_ARRVAL_P(deps_arr), v8js_persistent_zval_ctor);
}
// TODO: custom comparators
jsext->extension = new v8::Extension(jsext->name, jsext->source, jsext->deps_count, jsext->deps);
jsext->extension = new v8::Extension(ZSTR_VAL(jsext->name), ZSTR_VAL(jsext->source), jsext->deps_count, jsext->deps);
if (!zend_hash_add_ptr(v8js_process_globals.extensions, jsext->name, jsext)) {
v8js_jsext_free_storage(jsext);
#ifdef ZTS
v8js_process_globals.lock.unlock();
#endif
return FAILURE;
}
v8js_process_globals.extensions.insert(std::pair<std::string, v8js_jsext*>(key, jsext));
#ifdef ZTS
v8js_process_globals.lock.unlock();
@ -1021,9 +1002,6 @@ static PHP_METHOD(V8Js, registerExtension)
static PHP_METHOD(V8Js, getExtensions)
{
v8js_jsext *jsext;
ulong index;
zend_string *key;
zval *val, ext;
if (zend_parse_parameters_none() == FAILURE) {
return;
@ -1035,21 +1013,25 @@ static PHP_METHOD(V8Js, getExtensions)
v8js_process_globals.lock.lock();
#endif
if (v8js_process_globals.extensions) {
ZEND_HASH_FOREACH_KEY_VAL(v8js_process_globals.extensions, index, key, val) {
if (key) {
jsext = (v8js_jsext *) Z_PTR_P(val);
if (v8js_process_globals.extensions.size() > 0) {
for (auto &it: v8js_process_globals.extensions) {
zval ext;
jsext = it.second;
array_init(&ext);
add_assoc_bool_ex(&ext, ZEND_STRL("auto_enable"), jsext->auto_enable);
if (jsext->deps_ht) {
if (jsext->deps) {
zval deps_arr;
array_init(&deps_arr);
zend_hash_copy(Z_ARRVAL_P(&deps_arr), jsext->deps_ht, (copy_ctor_func_t) zval_add_ref);
int i;
array_init_size(&deps_arr, jsext->deps_count);
for (i = 0; i < jsext->deps_count; i++) {
zval z;
ZVAL_STRING(&z, jsext->deps[i]);
zend_hash_next_index_insert(Z_ARRVAL_P(&deps_arr), &z);
}
add_assoc_zval_ex(&ext, ZEND_STRL("deps"), &deps_arr);
}
add_assoc_zval_ex(return_value, ZSTR_VAL(key), ZSTR_LEN(key), &ext);
add_assoc_zval_ex(return_value, it.first.data(), it.first.size(), &ext);
}
} ZEND_HASH_FOREACH_END();
}
#ifdef ZTS

View File

@ -15,6 +15,8 @@
#ifndef V8JS_CLASS_H
#define V8JS_CLASS_H
/* Forward declarations */
struct v8js_jsext;
/* Abbreviate long type names */
typedef v8::Persistent<v8::FunctionTemplate, v8::CopyablePersistentTraits<v8::FunctionTemplate> > v8js_tmpl_t;
@ -88,6 +90,8 @@ struct v8js_ctx {
# define V8JS_TSRMLS_FETCH()
#endif
void v8js_jsext_free_storage(v8js_jsext *jsext);
static inline struct v8js_ctx *v8js_ctx_fetch_object(zend_object *obj) {
return (struct v8js_ctx *)((char *)obj - XtOffsetOf(struct v8js_ctx, std));
}