Apache crashes on shutdown when using pg_pconnect()

Description

Problem Scenario:

Start Apache (version 2.4.58).
Execute any PHP script that uses pg_pconnect function to establish a PostgreSQL persistent database connection (host= port= dbname= user=).
Stop Apache.

Observed Behavior:

After stopping Apache, the dump file is generated.
The dump file includes backtrace information:

EXCEPTION_RECORD:  (.exr -1)
ExceptionAddress: 00007ffe1399c5a0 (php_pgsql+0x000000000000c5a0)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 0000000000000008
   Parameter[1]: 00007ffe1399c5a0
Attempt to execute non-executable address 00007ffe1399c5a0
...
00000098`27eff2e8 00007ffe`0e8b048a     : 0000021e`610ff298 0000021e`61233070 0000021e`5c7cf510 0000021e`59870000 : php_pgsql+0xc5a0
00000098`27eff2f0 00007ffe`0e8828b2     : 00000000`00000000 00007ffe`00000000 00000000`00000168 00007ffe`313237eb : php8ts!zend_register_list_destructors_ex+0x10a
00000098`27eff320 00007ffe`0e7d47ca     : 00000000`00000009 00000000`00000009 00000000`00000168 0000021e`59870000 : php8ts!zend_hash_graceful_reverse_destroy+0x1d2
00000098`27eff370 00007ffe`0e947d52     : 00000000`00000000 00000000`00000000 0000021e`59919438 00007ffe`313237eb : php8ts!zend_append_version_info+0xcba
00000098`27eff3a0 00007ffe`0e7d1982     : 0000021e`59919458 00000000`5a27cb50 0000021e`59919438 00000000`00000000 : php8ts!ts_free_id+0xc2
00000098`27eff3f0 00007ffe`0e91bab6     : 0000021e`5c0be2b8 00000000`00000000 0000021e`5c0be168 00000000`00000000 : php8ts!zend_alloc_ce_cache+0x6c2
00000098`27eff420 00007ffe`0e91bb49     : 0000021e`5c0be1e0 0000021e`614951b0 00000000`00000000 0000021e`5987ae50 : php8ts!php_module_shutdown+0x36
00000098`27eff450 00007ffe`2a343288     : 0000021e`5987ae50 0000021e`5c0be188 0000021e`59894f70 00000000`5a27cb50 : php8ts!php_module_shutdown_wrapper+0x9
00000098`27eff480 00007ffe`0fbdffbe     : 0000021e`5987ae50 0000021e`599133e0 00000000`00000000 00000000`0000ea60 : php8apache2_4+0x3288
00000098`27eff4b0 00007ffe`0fbdf429     : 00000000`00000000 00000000`5a27cb50 00000000`0000ea60 00000000`5a27cb50 : libapr_1!apr_pool_userdata_get+0xe348
00000098`27eff4e0 00000000`5a23983c     : 00000000`00000000 00000000`5a27cb50 00000000`00000780 00000000`00000001 : libapr_1!apr_pool_userdata_get+0xd7b3
00000098`27eff510 00000000`5a23f59c     : 00000000`00000017 0000021e`5987ae50 0000021e`598c43f8 00000000`5a2512b6 : libhttpd!ap_get_server_revision+0x2766c
00000098`27eff5e0 00000000`5a243e3b     : 0000021e`5987ae50 0000021e`598dc0a8 00000000`00000000 0000021e`5988938f : libhttpd!ap_get_server_revision+0x2d3cc
00000098`27eff630 00007ff7`878922bb     : 00000000`5a297028 0000021e`598a2678 00000098`27eff6c9 0000021e`5989f2d0 : libhttpd!ap_get_server_revision+0x31c6b
00000098`27eff660 00007ff7`87893c28     : 00000000`00000003 0000021e`5988a730 00000000`00000064 00000000`00000000 : httpd+0x22bb
00000098`27eff730 00007ffe`31d6257d     : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : httpd+0x3c28
00000098`27eff770 00007ffe`33f8aa58     : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : kernel32!BaseThreadInitThunk+0x1d
00000098`27eff7a0 00000000`00000000     : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : ntdll!RtlUserThreadStart+0x28

Expected Outcome:

The script should execute without issues.

PHP Version

8.1.25

Operating System

Windows 11

would it be possible to test with a 8.2.x release and sees if this still crashes (or even a different backtrace) ?

would it be possible to test with a 8.2.x release and sees if this still crashes (or even a different backtrace) ?

I managed to reproduce the bug with PHP 8.2.31.
The backtrace is very similar.

EXCEPTION_RECORD:  (.exr -1)
ExceptionAddress: 00007ffe0f26c5a0 (php_pgsql+0x000000000000c5a0)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 0000000000000008
   Parameter[1]: 00007ffe0f26c5a0
Attempt to execute non-executable address 00007ffe0f26c5a0

STACK_TEXT:  
000000ab`72eff518 00007ffe`0e8b048a     : 00000278`9fda32a8 00000278`9fedc8a0 00000278`9b406c50 00000278`984e0000 : php_pgsql+0xc5a0
000000ab`72eff520 00007ffe`0e8828b2     : 00000000`00000000 00007ffe`00000000 00000000`00000168 00007ffe`313237eb : php8ts!zend_register_list_destructors_ex+0x10a
000000ab`72eff550 00007ffe`0e7d47ca     : 00000000`00000009 00000000`00000009 00000000`00000168 00000278`984e0000 : php8ts!zend_hash_graceful_reverse_destroy+0x1d2
000000ab`72eff5a0 00007ffe`0e947d52     : 00000000`00000000 00000000`00000000 00000278`985778b8 00007ffe`313237eb : php8ts!zend_append_version_info+0xcba
000000ab`72eff5d0 00007ffe`0e7d1982     : 00000278`985778d8 00000000`5a27cb50 00000278`985778b8 00000000`00000000 : php8ts!ts_free_id+0xc2
000000ab`72eff620 00007ffe`0e91bab6     : 00000278`9ad022d8 00000000`00000000 00000278`9ad02188 00000000`00000000 : php8ts!zend_alloc_ce_cache+0x6c2
000000ab`72eff650 00007ffe`0e91bb49     : 00000278`9ad02200 00000278`a1554180 00000000`00000000 00000278`984eac80 : php8ts!php_module_shutdown+0x36
000000ab`72eff680 00007ffe`2a383288     : 00000278`9ad02188 00000278`9ad021a8 00000278`98503630 00000000`5a27cb50 : php8ts!php_module_shutdown_wrapper+0x9
000000ab`72eff6b0 00007ffe`0f32ffbe     : 00000278`984eac80 00000278`9858f950 00000000`00000000 00000000`0000e54c : php8apache2_4+0x3288
000000ab`72eff6e0 00007ffe`0f32f429     : 00000000`00000000 00000000`5a27cb50 00000000`0000e54c 00000000`5a27cb50 : libapr_1!apr_pool_userdata_get+0xe348
000000ab`72eff710 00000000`5a23983c     : 00000000`00000000 00000000`5a27cb50 ffffffff`fff0bdc0 00000000`5a27cb50 : libapr_1!apr_pool_userdata_get+0xd7b3
000000ab`72eff740 00000000`5a23f59c     : 00000000`00000017 00000278`984eac80 00000278`98540528 00000000`5a2512b6 : libhttpd!ap_get_server_revision+0x2766c
000000ab`72eff810 00000000`5a243e3b     : 00000278`984eac80 00000278`98542128 00000000`00000000 00000278`98500aef : libhttpd!ap_get_server_revision+0x2d3cc
000000ab`72eff860 00007ff6`5ae322bb     : 00000000`5a297028 00000278`98510d38 000000ab`72eff8f9 00000278`9850d710 : libhttpd!ap_get_server_revision+0x31c6b
000000ab`72eff890 00007ff6`5ae33c28     : 00000000`00000003 00000278`984f79c0 00000000`00000064 00000000`00000000 : httpd+0x22bb
000000ab`72eff960 00007ffe`31d6257d     : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : httpd+0x3c28
000000ab`72eff9a0 00007ffe`33f8aa58     : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : kernel32!BaseThreadInitThunk+0x1d
000000ab`72eff9d0 00000000`00000000     : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : ntdll!RtlUserThreadStart+0x28

So just to confirm, the crashes occurs only with persistent connection (when the extension tries to register the connection cleanup callback) ?

So just to confirm, the crashes occurs only with persistent connection (when the extension tries to register the connection cleanup callback) ?

yes, with pgsql.allow_persistent=0 in php.ini the crash doesn't occur.

Is there any other extension involved loaded eventually ? If so if you disable them do you still see the crashes ?

Is there any other extension involved loaded eventually ? If so if you disable them do you still see the crashes ?

I tried disabling every extension except pg_pgsql.dll, but the crash still occurs.

"unfortunately", cannot reproduce under Linux.

By any chance @nielsdos, do you have any idea what s going on with apache ?

I can reproduce this.

Script:

<?php
echo "hi";
$conn = pg_pconnect("dbname=php");
var_dump($conn);

Ran Apache on Linux using:
sudo DEBUGINFOD_URLS="https://debuginfod.archlinux.org" valgrind httpd -DFOREGROUND -k start

Surfed to the page 5 times in my browser (don't know if the amount makes a difference).

Got this:

==297454== Invalid read of size 8
==297454==    at 0x537DA9B: _close_pgsql_plink (pgsql.c:317)
==297454==    by 0x56A2D51: plist_entry_destructor (zend_list.c:196)
==297454==    by 0x569C232: _zend_hash_del_el_ex (zend_hash.c:1425)
==297454==    by 0x569C32D: _zend_hash_del_el (zend_hash.c:1452)
==297454==    by 0x569E0E2: zend_hash_graceful_reverse_destroy (zend_hash.c:1977)
==297454==    by 0x56A2E9E: zend_destroy_rsrc_list (zend_list.c:234)
==297454==    by 0x567DB9A: executor_globals_dtor (zend.c:816)
==297454==    by 0x55B4509: ts_free_id (TSRM.c:560)
==297454==    by 0x567E615: zend_shutdown (zend.c:1127)
==297454==    by 0x55BB344: php_module_shutdown (main.c:2426)
==297454==    by 0x55BB301: php_module_shutdown_wrapper (main.c:2397)
==297454==    by 0x580EC48: php_apache_child_shutdown (sapi_apache2.c:433)
==297454==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==297454== 
==297454== 
==297454== Process terminating with default action of signal 11 (SIGSEGV)
==297454==    at 0x537DA9B: _close_pgsql_plink (pgsql.c:317)
==297454==    by 0x56A2D51: plist_entry_destructor (zend_list.c:196)
==297454==    by 0x569C232: _zend_hash_del_el_ex (zend_hash.c:1425)
==297454==    by 0x569C32D: _zend_hash_del_el (zend_hash.c:1452)
==297454==    by 0x569E0E2: zend_hash_graceful_reverse_destroy (zend_hash.c:1977)
==297454==    by 0x56A2E9E: zend_destroy_rsrc_list (zend_list.c:234)
==297454==    by 0x567DB9A: executor_globals_dtor (zend.c:816)
==297454==    by 0x55B4509: ts_free_id (TSRM.c:560)
==297454==    by 0x567E615: zend_shutdown (zend.c:1127)
==297454==    by 0x55BB344: php_module_shutdown (main.c:2426)
==297454==    by 0x55BB301: php_module_shutdown_wrapper (main.c:2397)
==297454==    by 0x580EC48: php_apache_child_shutdown (sapi_apache2.c:433)

I don't know if I'll still have time to look today; otherwise tomorrow.

Your fix makes sense, asan should not complain I think here ; hopefully will fix windows' case.

I dug deeper into the rabbit hole. I think all persistent resources on ZTS are affected.
This is not specific to Windows, nor pgsql.

On shutdown the following happens:
https://github.com/php/php-src/blob/master/Zend/zend.c#L1124-L1125 gets executed, which first destroys the persistent resources and then destroys the modules. Note that after those two lines are executed, the module globals are freed and the module is unloaded if it was loaded dynamically.

However, there may still be persistent resources loaded, which will be freed by

php-src/Zend/zend.c

Line 1160 in c5fbcfa

ts_free_id(executor_globals_id);

which will call executor_globals_dtor which will execute this:

php-src/Zend/zend.c

Lines 833 to 835 in c5fbcfa

    
           if (&executor_globals->persistent_list != global_persistent_list) { 
        
           	zend_destroy_rsrc_list(&executor_globals->persistent_list); 
        
           }

Here we see that there can still be persistent resources destroyed after either the module globals have been destroyed (which was the original issue I found), or after the module is unloaded (the remaining issue).

I thought of this fix: https://gist.github.com/nielsdos/e21360b9c100d23c1ae40af3ef998ee6 but I'm not sure if it's right. In particular, I'm afraid of this because we have bad ZTS test coverage and no Apache test coverage at all.
Also, I'm not 100% sure I understand the "true global" global_persistent_list correctly. In any case, I tested this in different configurations and it seems to work.
I've made a PR on my fork to run tests: ndossche#87 we'll see how that goes before moving forward.

@iluuu1994

Scratch that, my proposed patch won't work of course because we still need to destroy all persistent lists of all threads via the ZTS dtor, which I removed.
I'm out of ideas on how to fix this to be honest. I think we may only destroy the modules after the executor has been shut down such that all persistent resources are destroyed by the time modules are destroyed. However, changing destruction order will probably cause other problems because assumptions of modules can be broken.
cc @iluuu1994 Could you please help here when you have some time?

Another idea that keeps BC would be to keep the current ifdef ZTS + module_started fix, and split the module destruction into a destroy phase and an unload phase, performing the unload only after the executor globals have been destroyed.
I think BC-wise that should be okay. We'd just have to think of a way to do the unloading after destruction in a nice way.

Here's that idea in patch form: https://gist.github.com/nielsdos/338774e915a1b528eaa8bd68ba7cc54c
I tested this with both static and dynamically loaded postgres and it works in both cases to resolve the issue.
As this does not touch the public APIs, and does not change destruction orderings, I think this is BC-safe.
Note that I removed the module_registry_unload_temp declaration in the header and replaced it with module_registry_unload because module_registry_unload_temp doesn't actually seem to exist. Neither of these APIs is public BTW.

Fix ZTS crashes with persistent resources in modules

Fix ZTS crashes with persistent resources in modules (#13381)

Navigation Menu

Repository navigation

Apache crashes on shutdown when using pg_pconnect() #12974

Description

PHP Version

Operating System

20 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (&executor_globals->persistent_list != global_persistent_list) {
	zend_destroy_rsrc_list(&executor_globals->persistent_list);
	}

Apache crashes on shutdown when using pg_pconnect() #12974

Description

Description

PHP Version

Operating System

Activity

devnexen commented on Dec 19, 2023

stable-staple commented on Dec 19, 2023

devnexen commented on Dec 19, 2023

stable-staple commented on Dec 19, 2023

devnexen commented on Dec 19, 2023

stable-staple commented on Dec 19, 2023

devnexen commented on Dec 19, 2023

devnexen commented on Dec 20, 2023

ndossche commented on Dec 20, 2023

20 remaining items

devnexen commented on Feb 8, 2024

ndossche commented on Feb 8, 2024

ndossche commented on Feb 8, 2024

ndossche commented on Feb 10, 2024

ndossche commented on Feb 11, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions