[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cli server crashes on SIGINT when compiled with ZEND_RC_DEBUG=1 #11716

Closed
ju1ius opened this issue Jul 16, 2023 · 4 comments
Closed

cli server crashes on SIGINT when compiled with ZEND_RC_DEBUG=1 #11716

ju1ius opened this issue Jul 16, 2023 · 4 comments
Assignees
@arnaud-lb
Labels
Bug SAPI: cli_server Status: Verified

Comments

@ju1ius
Copy link
Contributor
ju1ius commented Jul 16, 2023

Description

Steps to reproduce:

  • PHP must be compiled with --enable-debug and CFLAGS="-DZEND_RC_DEBUG=1"
  • launch a cli server like php -S localhost:8888
  • terminate the process with a SIGINT (i.e. CTRL+C)

Here's the gdb backtrace:

__pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
0x00007fd65fdba15f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
0x00007fd65fd6c472 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
0x00007fd65fd564b2 in __GI_abort () at ./stdlib/abort.c:79
0x00007fd65fd563d5 in __assert_fail_base (fmt=0x7fd65fecadc8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
    assertion=assertion@entry=0x5582e4bcde80 "(zval_gc_flags((p)->u.type_info) & ((1<<7)|(1<<8))) != (1<<7)",
    file=file@entry=0x5582e4bcde18 "/usr/src/php-src/Zend/zend_types.h", line=line@entry=1289,
    function=function@entry=0x5582e4bce468 <__PRETTY_FUNCTION__.62> "zend_gc_delref") at ./assert/assert.c:92
0x00007fd65fd653a2 in __assert_fail (
    assertion=0x5582e4bcde80 "(zval_gc_flags((p)->u.type_info) & ((1<<7)|(1<<8))) != (1<<7)",
    file=0x5582e4bcde18 "/usr/src/php-src/Zend/zend_types.h", line=1289,
    function=0x5582e4bce468 <__PRETTY_FUNCTION__.62> "zend_gc_delref") at ./assert/assert.c:101
0x00005582e4867e74 in zend_gc_delref (p=0x5582e52c8210) at /usr/src/php-src/Zend/zend_types.h:1289
0x00005582e4868109 in zend_string_release (s=0x5582e52c8210) at /usr/src/php-src/Zend/zend_string.h:345
 0x00005582e486e022 in zend_hash_destroy (ht=0x5582e50237a8 <server+648>) at /usr/src/php-src/Zend/zend_hash.c:1738
0x00005582e49d7e23 in php_cli_server_dtor (server=0x5582e5023520 <server>) at /usr/src/php-src/sapi/cli/php_cli_server.c:2333
0x00005582e49d8f82 in do_cli_server (argc=4, argv=0x5582e5150770) at /usr/src/php-src/sapi/cli/php_cli_server.c:2864
0x00005582e49cefeb in main (argc=4, argv=0x5582e5150770) at /usr/src/php-src/sapi/cli/php_cli.c:1340

Looks like this assertion:

ZEND_ASSERT((zval_gc_flags((p)->u.type_info) & (GC_PERSISTENT|GC_PERSISTENT_LOCAL)) != GC_PERSISTENT);

fails when calling

zend_hash_destroy(&server->extension_mime_types);

PHP Version

php8.3-dev

Operating System

debian sid
@nielsdos
Copy link
Member
nielsdos commented Jul 16, 2023 •
edited
Loading

I guess this works:

diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c
index a8571af75c..4cdf681761 100644
--- a/Zend/zend_hash.c
+++ b/Zend/zend_hash.c
@@ -898,6 +898,11 @@ static zend_always_inline zval *_zend_hash_str_add_or_update_i(HashTable *ht, co
 	ht->nNumOfElements++;
 	p = ht->arData + idx;
 	p->key = key = zend_string_init(str, len, GC_FLAGS(ht) & IS_ARRAY_PERSISTENT);
+#if ZEND_RC_DEBUG
+	if (GC_FLAGS(ht) & GC_PERSISTENT_LOCAL) {
+		GC_MAKE_PERSISTENT_LOCAL(key);
+	}
+#endif
 	p->h = ZSTR_H(key) = h;
 	HT_FLAGS(ht) &= ~HASH_FLAG_STATIC_KEYS;
 	if (flag & HASH_LOOKUP) {
diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c
index 9c71b83580..5bcf736969 100644
--- a/sapi/cli/php_cli_server.c
+++ b/sapi/cli/php_cli_server.c
@@ -2320,6 +2320,7 @@ static void php_cli_server_mime_type_ctor(php_cli_server *server, const php_cli_
 	const php_cli_server_ext_mime_type_pair *pair;
 
 	zend_hash_init(&server->extension_mime_types, 0, NULL, NULL, 1);
+	GC_MAKE_PERSISTENT_LOCAL(&server->extension_mime_types);
 
 	for (pair = mime_type_map; pair->ext; pair++) {
 		size_t ext_len = strlen(pair->ext);

CLI is single threaded, so clearly that hashtable is also local.
And also I think that persistent local hashtables should mark the strings they create as persistent local as well, that seems to make sense. Since GC_PERSISTENT_LOCAL is only used for debugging I guarded it with an #if.
But this is a bit of a shot in the dark, so I'd like confirmation from someone more accustomed to Zend.

@Girgias
Copy link
Member
Girgias commented Jul 16, 2023

Assigning @arnaud-lb as they helped me figure out this sort of issue with strings last time I was doing something with the CLI

@arnaud-lb
Copy link
Member

@nielsdos this looks good. Do you want to make a PR? Otherwise, I will do. We may have the same issue with req->headers and req->headers_original_case in php_cli_server_request_ctor().

The cli server appears to be the only place where we destroy persistent hash tables before zend_rc_debug is set to false by php_module_shutdown(). This explains why we didn't need to take care of GC_PERSISTENT_LOCAL before in zend_hash.

@nielsdos
Copy link
Member

@arnaud-lb Thank you for checking. I can create the PR soon and check the other cases.

nielsdos added a commit to nielsdos/php-src that referenced this issue Jul 21, 2023
@nielsdos
Fix phpGH-11716: cli server crashes on SIGINT when compiled with ZEND…
b0a7b51 
…_RC_DEBUG=1
@nielsdos nielsdos mentioned this issue Jul 21, 2023
Closed
nielsdos added a commit to nielsdos/php-src that referenced this issue Jul 21, 2023
@nielsdos
Fix phpGH-11716: cli server crashes on SIGINT when compiled with ZEND…
3a2b824 
…_RC_DEBUG=1
nielsdos added a commit that referenced this issue Jul 21, 2023
@nielsdos
Merge branch 'PHP-8.1' into PHP-8.2
272dc9a 
* PHP-8.1:
  Fix GH-11716: cli server crashes on SIGINT when compiled with ZEND_RC_DEBUG=1
nielsdos added a commit that referenced this issue Jul 21, 2023
@nielsdos
Merge branch 'PHP-8.2'
d58e3c0 
* PHP-8.2:
  Fix GH-11716: cli server crashes on SIGINT when compiled with ZEND_RC_DEBUG=1
@timnolte timnolte mentioned this issue Sep 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arnaud-lb arnaud-lb

Labels
Bug SAPI: cli_server Status: Verified
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ju1ius @arnaud-lb @nielsdos @Girgias