Description
I stopped using registerExtension, but I think there might be one more registerExtension bug anyway, or even non-obvious bugs.
- From MSHUTDOWN_FUNCTION, the only thing I can think of that would leak are extensions and their associated data (e.g. deps).
Motivation for suggesting switching all uses of zend_string_dup/zend_string_copy to zend_string_init() instead
- I don't expect copying strings vs adding references to be a large source of extra memory usage for v8js.
- If this crashes or misbehaves (e.g. frees a string twice, or too early, or not at all), it would be easier to narrow down the cause when running with VALGRIND.
- Interned or temporary strings can cause problem if they're persisted by v8js beyond the lifetime of a request.
» grep -E 'zend_string_(copy|dup)' *.cc *.h
v8js_class.cc: Z_STR_P(p) = zend_string_dup(Z_STR_P(p), 1);
v8js_class.cc: // (zend_string_dup would return the original interned string, if interned, so we don't use that)
v8js_v8object_class.cc: f->common.function_name = zend_string_copy(method);
v8js_variables.cc: ctx->variable_name = zend_string_copy(Z_STR_P(item));
One use from registerExtension with an array of deps seems like it might also be a problem for non-ZTS builds.
Because of that, zend_persistent_zval_dup should probably use zend_string_init instead. zend_string_dup() will return the original pointer for interned strings (e.g. in php 7.2)
// Allocate a persistent string which will survive until module shutdown on both ZTS(Persistent) and NTS(Not interned, those would be cleaned up)
// (zend_string_dup would return the original interned string, if interned, so we don't use that)
jsext->name = zend_string_init(ZSTR_VAL(name), ZSTR_LEN(name), 1);
jsext->source = zend_string_init(ZSTR_VAL(source), ZSTR_LEN(source), 1);
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);
}Context for why I'm looking at string copying in V8js (if anyone has similar issues): I'm seeing a segfault in zend_interned_string_find_permanent in long-running httpd processes for a fraction of apache restarts (if I read the instruction pointer correctly).
- I'm not using registerExtension anywhere
- I'm using php 7.2.24 and v8js 2.1.0. Some opcache bugs were fixed in 7.2.25 and 7.3, but they all seem unrelated.
- The code in question uses V8Js with instance properties that are strings, PHP closures with use($var), etc.
- Some V8js instances execute multiple scripts, then have methods invoked multiple times. I haven't obtained a core dump yet. Rolling back V8js seems to have helped in 7.1
- This happens with opcache enabled, and with/without xdebug
- There are two apache(httpd) pools - the segfault only seems to happen during restarts after the second pool gets stopped, not the first. This might be a coincidence.
zend_interned_string_find_permanent seems like it would only get called by opcache in php 7.2. The line it crashes on suggests that the pointer to the zend_string that is being looked up was corrupted.
0000000000474110 <zend_interned_string_find_permanent>:
474110: 41 57 push %r15
474112: 49 89 ff mov %rdi,%r15
474115: 41 56 push %r14
474117: 41 55 push %r13
474119: 41 54 push %r12
47411b: 55 push %rbp
47411c: 53 push %rbx
47411d: 48 83 ec 08 sub $0x8,%rsp
474121: 48 8b 6f 08 mov 0x8(%rdi),%rbp <- crashes here, probably fetching zend_string_hash_val(str) for an invalid str pointer
474125: 48 85 ed test %rbp,%rbp
474128: 74 7e je 4741a8 <zend_interned_string_find_permanent+0x98>
....EDIT: I obtained a core dump. I was mistaken about what called zend_interned_string_find_permanent(). The source before signal_handler can vary (memcached, curl, reading files, etc), but it's just any point when signal_handler can get called for the httpd stop.
For gdb /path/to/libphp7.so path/to/core_dump
Program terminated with signal 11, Segmentation fault.
...
(gdb) bt
#0 0x00007f3e483ca121 in zend_interned_string_find_permanent () from /usr/local/php/modules/libphp7.so
#1 0x00007f3e3d0679f9 in accel_shutdown () from /usr/local/php/lib/php/20170718/opcache.so
#2 0x00007f3e3d069ff0 in zm_shutdown_zend_accelerator () from /usr/local/php/lib/php/20170718/opcache.so
#3 0x00007f3e483a3ae3 in module_destructor () from /usr/local/php/modules/libphp7.so
#4 0x00007f3e4839b3cc in module_destructor_zval () from /usr/local/php/modules/libphp7.so
#5 0x00007f3e483afa48 in zend_hash_graceful_reverse_destroy () from /usr/local/php/modules/libphp7.so
#6 0x00007f3e4839d1ce in zend_shutdown () from /usr/local/php/modules/libphp7.so
#7 0x00007f3e4832f99b in php_module_shutdown () from /usr/local/php/modules/libphp7.so
#8 0x00007f3e4832fa59 in php_module_shutdown_wrapper () from /usr/local/php/modules/libphp7.so
#9 0x00007f3e48466e51 in php_apache_child_shutdown () from /usr/local/php/modules/libphp7.so
#10 0x00007f3e503011ae in apr_pool_destroy () from /lib64/libapr-1.so.0
#11 0x00007f3e4ada823c in clean_child_exit () from /etc/httpd/modules/mod_mpm_prefork.so
#12 0x00007f3e4ada827b in just_die () from /etc/httpd/modules/mod_mpm_prefork.so
#13 0x00007f3e483ca30d in zend_signal_handler () from /usr/local/php/modules/libphp7.so
#14 0x00007f3e483ca45b in zend_signal_handler_defer () from /usr/local/php/modules/libphp7.so
#15 <signal handler called>
#16 0x00007f3e500da6f0 in __read_nocancel () from /lib64/libpthread.so.0
#17 0x00007f3e4834d079 in php_stdiop_read () from /usr/local/php/modules/libphp7.so
#18 0x00007f3e4834770c in _php_stream_fill_read_buffer () from /usr/local/php/modules/libphp7.so
#19 0x00007f3e48347ba7 in _php_stream_get_line () from /usr/local/php/modules/libphp7.so
#20 0x00007f3e4829cb03 in php_exec () from /usr/local/php/modules/libphp7.so
#21 0x00007f3e4829ce7a in zif_exec () from /usr/local/php/modules/libphp7.so
#22 0x00007f3e4845a8ec in execute_ex () from /usr/local/php/modules/libphp7.so
#23 0x00007f3e48465664 in zend_execute () from /usr/local/php/modules/libphp7.so
#24 0x00007f3e4839d830 in zend_execute_scripts () from /usr/local/php/modules/libphp7.so
#25 0x00007f3e4832fcf0 in php_execute_script () from /usr/local/php/modules/libphp7.so
#26 0x00007f3e484680ca in php_handler () from /usr/local/php/modules/libphp7.so
#27 0x000055c9bc02ca40 in ?? ()
#28 0x000055c9bc75ca30 in ?? ()
#29 0x000055c9bc3f4be0 in ?? ()
#30 0x000055c9bc6e5ad0 in ?? ()
#31 0x000055c9bc02cf89 in ?? ()
#32 0x000055c9bc40bad8 in ?? ()
#33 0x000055c9bc025c4f in ?? ()
#34 0x000000005e0fbb63 in ?? ()
#35 0x951cbc1dbc51bb00 in ?? ()
#36 0x000055c9bc75dfc6 in ?? ()
#37 0x000055c9bc75ca30 in ?? ()
#38 0x0000000000000000 in ?? ()
EDIT: The larger problem is that this is a hard restart instead of a graceful restart, so the details of shutdown are less important.