Avoid uses of zend_string_dup/zend_string_copy when it will outlive the http request

[TysonAndre]

Related to #255 and #247

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.

[TysonAndre]

v8js/v8js_class.cc

Lines 793 to 798 in 461230b

	static void v8js_persistent_zval_ctor(zval *p) /* {{{ */
	{
	assert(Z_TYPE_P(p) == IS_STRING);
	Z_STR_P(p) = zend_string_dup(Z_STR_P(p), 1);
	}
	/* }}} */

E.g. the helper returns s, even if s was a temporary interned string (allocated with emalloc)

static zend_always_inline zend_string *zend_string_dup(zend_string *s, bool persistent)
{
	if (ZSTR_IS_INTERNED(s)) {
		return s;
	} else {
		return zend_string_init(ZSTR_VAL(s), ZSTR_LEN(s), persistent);
	}
}