Re: tsrm_shutdown() and the CLI SAPI

From: Date: Fri, 02 Jun 2006 05:46:45 +0000
Subject: Re: tsrm_shutdown() and the CLI SAPI
References: 1  Groups: php.internals 
Request: Send a blank email to internals+get-23862@lists.php.net to get a copy of this message
Hi Dmitry, Have some diagnostics: 1) zend_shutdown() is called before anything in TSRM. It invokes zend_hash_graceful_reverse_destroy(&module_registry); 2) module_destructor() is invoked via the hash-destroyer (being part of the module_registry hash constructor). This performs the MSHUTDOWN routine for each registered module, which in several cases (now) includes a PHP-side call to ts_free_id(). The ts_free_id() call usually allows the next stage to be unproblematic; it also marks the module's entry in the resource table as 'done', so that the final clearing loop won't try to double-free it. However, ts_free_id() doesn't check for 'done' itself before it frees resources. 3) DL_UNLOAD (an alias for FreeLibrary, under doze) is then called on any module loaded via dl() or php.ini (checking for a module handle)... it looks like this stage might only have happened under Macs on CLI until mid-March... 4) tsrm_shutdown() - the final clearing loop - is called via the end of the various SAPI source files (in this case, php_cli.c), and any resource that hasn't been freed explicitly by this point is freed there. It won't help our particular situation, but I think automatically calling ts_free_id() on dl-loaded modules would be better than either blanket avoidance of tsrm_shutdown() or requiring each module to call ts_free_id() explicitly during MINIT. Patch(es) to do that in PHP_5_2 branch attached. The problem in PHP-GTK is we're crashing when the compiler_globals dtor is called in tsrm_shutdown(), rather than when the gtk_globals dtor is called. I think this is a ZE problem rather than ours, but see what you think: somehow, in compiler_globals_dtor(), compiler_globals->class_table != GLOBAL_CLASS_TABLE and the compiler_globals->class_table is destroyed and freed because of that. - Steph ----- Original Message ----- From: "Dmitry Stogov" <dmitry@zend.com> To: "'Steph Fox'" <steph@zend.com> Cc: "'internals'" <internals@lists.php.net> Sent: Thursday, June 01, 2006 1:25 PM Subject: RE: [PHP-DEV] tsrm_shutdown() and the CLI SAPI
Patch was cutted. Dmitry.
-----Original Message----- From: Dmitry Stogov [mailto:dmitry@zend.com] Sent: Thursday, June 01, 2006 3:05 PM To: 'Andi Gutmans'; 'Steph Fox'; 'Frank M. Kromann' Cc: 'internals'; 'Antony Dovgal'; 'Xuefer' Subject: RE: [PHP-DEV] tsrm_shutdown() and the CLI SAPI Hi Stepth, I reproduced crash of php-gtk on linix with php-zts. I mean running "php -v" then php.ini contains "extension=gtk2.so". The crash occurs only then php compiled with --enable-debug. The reason of the crash is a bug in ZE that is activated by memory leaks in php-gtk. In case of "php -v" we don't do request startup/shutdown and as result we don't handle memory leaks on request shutdown. During tsrm_shutdown() shutdow_nmemory_manager() is called after sapi_shutdown() and as result we cannot access sapi_globals then trying to print information about memory leaks and crashes. The solution - don't print information about memory leaks during tsrm_shutdown(). Patch for PHP_5_2 is attached, but it can be manually applied to any version. Steph, does you crash(es) go away with this patch? I am going to commit patch in 24h. Any objections? Thanks. Dmitry.
-----Original Message----- From: Andi Gutmans [mailto:andi@zend.com] Sent: Thursday, June 01, 2006 10:29 AM To: Steph Fox; Frank M. Kromann Cc: 'internals'; 'Antony Dovgal'; Dmitry Stogov; 'Xuefer' Subject: Re: [PHP-DEV] tsrm_shutdown() and the CLI SAPI As we are not planning to release a new version within the next couple of weeks, I suggest before jumping to conclusions we take a look at it. If you really need to comment out that line in the meanwhile that's OK with me. Andi At 09:50 PM 5/31/2006, Steph Fox wrote:
Yes, it would, given the root cause - but would you
really want to
break the whole of PHP for an academic exercise?
It's not really an academic exercise. If we know there's a bug someplace we should at least look into it and try and
understand it. Frank's referring to Zeev's three-years-ago decision to simply opt out of tsrm_shutdown() here... he's suggesting we revert it.
Then if we decide to remove the trsm_shutdown call for a
good reason
(circular dependency, blah blah blah) then we can do that
and put a
nice fat comment on why it's the right thing to do. But I
do think
it's benefical to try and understand what's happening.
Fine, but breaking working code while you're trying to understand what's happening is far from beneficial to our users. Can't
we at least #0 it?
Andi
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
__________ NOD32 1.1380 (20060125) Information __________ This message was checked by NOD32 antivirus system. http://www.eset.com
--------------------------------------------------------------------------------
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php __________ NOD32 1.1380 (20060125) Information __________ This message was checked by NOD32 antivirus system. http://www.eset.com


Index: Zend/zend_API.c =================================================================== RCS file: /repository/ZendEngine2/zend_API.c,v retrieving revision 1.296.2.27.2.13 diff -u -r1.296.2.27.2.13 zend_API.c --- Zend/zend_API.c 27 May 2006 20:16:53 -0000 1.296.2.27.2.13 +++ Zend/zend_API.c 2 Jun 2006 05:27:09 -0000 @@ -32,6 +32,7 @@ /* these variables are true statics/globals, and have to be mutex'ed on every access */ static int module_count=0; +static int dl_module_count=0; ZEND_API HashTable module_registry; /* this function doesn't check for too many parameters */ @@ -1831,6 +1832,8 @@ void module_destructor(zend_module_entry *module) { + int table_size; + TSRMLS_FETCH(); if (module->type == MODULE_TEMPORARY) { @@ -1852,6 +1855,8 @@ #if HAVE_LIBDL || defined(HAVE_MACH_O_DYLD_H) #if !(defined(NETWARE) && defined(APACHE_1_BUILD)) if (module->handle) { + table_size = tsrm_table_size() + 1; + ts_free_id(table_size - zend_dl_module_reverse()); DL_UNLOAD(module->handle); } #endif @@ -1899,6 +1904,12 @@ return ++module_count; } +/* return the dl module count */ +int zend_dl_module_reverse(void) +{ + return ++dl_module_count; +} + static zend_class_entry *do_register_internal_class(zend_class_entry *orig_class_entry, zend_uint ce_flags TSRMLS_DC) { zend_class_entry *class_entry = malloc(sizeof(zend_class_entry)); Index: Zend/zend_API.h =================================================================== RCS file: /repository/ZendEngine2/zend_API.h,v retrieving revision 1.207.2.8.2.2 diff -u -r1.207.2.8.2.2 zend_API.h --- Zend/zend_API.h 25 May 2006 10:01:06 -0000 1.207.2.8.2.2 +++ Zend/zend_API.h 2 Jun 2006 05:12:12 -0000 @@ -160,6 +160,7 @@ #endif int zend_next_free_module(void); +int zend_dl_module_reverse(void); BEGIN_EXTERN_C() ZEND_API int zend_get_parameters(int ht, int param_count, ...); Index: TSRM/TSRM.c =================================================================== RCS file: /repository/TSRM/TSRM.c,v retrieving revision 1.68.2.3 diff -u -r1.68.2.3 TSRM.c --- TSRM/TSRM.c 14 Mar 2006 15:16:07 -0000 1.68.2.3 +++ TSRM/TSRM.c 2 Jun 2006 05:06:49 -0000 @@ -542,7 +542,7 @@ while (p) { if (p->count > j && p->storage[j]) { - if (resource_types_table && resource_types_table[j].dtor) { + if (resource_types_table && resource_types_table[j].dtor && !resource_types_table[j].done) { resource_types_table[j].dtor(p->storage[j], &p->storage); } free(p->storage[j]); @@ -586,6 +586,12 @@ #endif } +/* Obtain the current resource table size */ +TSRM_API int tsrm_table_size(void) +{ + return id_count; +} + /* Allocate a mutex */ TSRM_API MUTEX_T tsrm_mutex_alloc(void) Index: TSRM/TSRM.h =================================================================== RCS file: /repository/TSRM/TSRM.h,v retrieving revision 1.50.2.2 diff -u -r1.50.2.2 TSRM.h --- TSRM/TSRM.h 14 Mar 2006 15:16:07 -0000 1.50.2.2 +++ TSRM/TSRM.h 2 Jun 2006 05:06:43 -0000 @@ -127,6 +127,7 @@ /* utility functions */ TSRM_API THREAD_T tsrm_thread_id(void); +TSRM_API int tsrm_table_size(void); TSRM_API MUTEX_T tsrm_mutex_alloc(void); TSRM_API void tsrm_mutex_free(MUTEX_T mutexp); TSRM_API int tsrm_mutex_lock(MUTEX_T mutexp);
« previous php.internals (#23862) next »