tsrm_shutdown() and the CLI SAPI

Thanks Xuefer...

This bug's been extant for a long time, and I only found out why when I
spent two days/nights trying to track down its history and mechanics.

It's a pig.

• Steph

----- Original Message -----
From: "Xuefer" xuefer@gmail.com
To: "Steph Fox" steph@zend.com
Cc: "internals" internals@lists.php.net
Sent: Sunday, May 28, 2006 4:58 AM
Subject: Re: [PHP-DEV] tsrm_shutdown() and the CLI SAPI

i can confirm this on other extension.
something like this
grep free_id /.c -B1 -A3
mbstring/mbstring.c-#ifdef ZTS
mbstring/mbstring.c: ts_free_id(mbstring_globals_id);
mbstring/mbstring.c-#else
mbstring/mbstring.c- _php_mb_globals_dtor(&mbstring_globals TSRMLS_CC);
mbstring/mbstring.c-#endif
have no problem with it
while some modules like
$ grep 'ndef ZTS' /.c -A2
apc/php_apc.c:#ifndef ZTS
apc/php_apc.c- php_apc_shutdown_globals(&apc_globals);
apc/php_apc.c-#endif

eaccelerator/eaccelerator.c:#ifndef ZTS
eaccelerator/eaccelerator.c-
eaccelerator_globals_dtor(&eaccelerator_globals TSRMLS_CC);
eaccelerator/eaccelerator.c-#endif
when compiled as shared module, will crash
eaccelerator(mmcache) workaround it by disabling the dtor.
/*??? FIXME
ZEND_INIT_MODULE_GLOBALS(eaccelerator, eaccelerator_init_globals,
eaccelerator_globals_dtor);
*/
ZEND_INIT_MODULE_GLOBALS(eaccelerator, eaccelerator_init_globals, NULL);

Hi,

Yes, something broke while we were at 5.1.3-dev.

It would be really nice if Dmitry could have a look at this. We have
many reports that things have gotten much more ustable on Windows since
5.1.2.

Edin

Steph Fox wrote:

Hi all,

I've spent a fun weekend debugging TSRM and bits of ZE2, having finally
figured that the zend_compiler_globals dtor was the point of failure here.

TSRM needs to mark resources as 'done' following a free and doesn't in
most cases, but that's actually not the main problem we have in PHP-GTK
(although fixing it may well solve everybody else's, I'll check that on
my travels).

Dmitry's "Optimized cleanup loops on request shutdown" commit(s)
spanning the 13th and 14th March broke Andrei's 'EG(full_tables_cleanup)
= 1' ruse (we needed user classes to be cleaned and internal classes to
stay), but it also broke TSRM shutdown completely for us. PHP-GTK
classes are recognized as being internal, but only the first one ever
reaches the dtor - even when there are no user-defined classes.

zend_hash_reverse_apply() is called at that point, so I'm guessing
there's a reentrancy issue behind my access violation crash.

I'll get back to this when I get some time, in the meantime if anyone
(Dmitry?) wants to play with this you need to know that I can't put the
5_2 compat stuff into CVS until Andrei OKs it, so PHP-GTK HEAD has to be
built against PHP_5_1 branch at present (anything after March 14th will
do).

• Steph

Under CLI ZTS build, when loading a PHP-GTK 2 .dll built against
anything this side of PHP 5.1.2 release, I'm seeing an 'access
violation' crash on reaching:

 if (resource_types_table && !resource_types_table[j].done &&

resource_types_table[j].dtor) {
resource_types_table[j].dtor(p->storage[j], &p->storage);
}

Similar code is also used in:

tsrm_free_interpreter_context()
ts_free_thread()
ts_free_worker_threads()
ts_free_id()

I thought ts_free_thread() was crashing at that line too at first, but
that turned out to be because there's no 'done' check there, i.e.
double dtors are possible via some functions. It's still vaguely
possible there's a double flypast causing the crash I'm seeing, but I
have everything I know about protected now. (On my laptop that is.)

Eventually I found the resource id for PHP-GTK - the only extension
I'm loading during runtime, via php.ini - is 32nd out of a possible

32. "Eventually", because that means I can't use ts_free_id() to avoid
    the crash as advised by Frank (and Tony, and Dmitry, and anyone else
    who ever used that MSHUTDOWN workaround for CLI). Interesting too,
    because the resource that causes the crash appears to be something
    completely other. Tracking down resource id #5 - and that's all I know
    about it apart from it crashes - is a barrel of laughs, I'll let y'all
    know if I ever find out which of the many possible extensions/files in
    ext/standard or core it is.

Short version: One line is problematic, and only in one function, and
presumably only under CLI SAPI. (CGI already doesn't call
tsrm_shutdown(), thanks to similar issues some 3 years ago).

The question is whether to take 'the Zeev approach' and simply comment
out the tsrm_shutdown() call at the end of sapi/cli/php_cli.c, or
whether to spend time knitting up a fine-grained approach to prevent
the one bad line in the function from being called in single-threaded
environments? I think I've given up on the idea of actually resolving
the bug now...

Thoughts?

• oh, and don't make one of them 'chicken-and-egg' - this is
  definitively NOT related to the shutdown order in main.c!!!!!

• Steph

--

__________ NOD32 1.1380 (20060125) Information __________

This message was checked by NOD32 antivirus system.
http://www.eset.com

Hello Steph,

can it be that the return value of the dtor functions are wrong? They
have always been wrong and during last months we tried to sort things
out. Actually i was planning to have HEAD and 5.2 both respect the APPLY
values correct. Should i wait with those changes now until someone figures
out this dtor issue or go ahead and we look then?

best regards
marcus

Monday, May 29, 2006, 11:40:15 AM, you wrote:

Hi all,

I've spent a fun weekend debugging TSRM and bits of ZE2, having finally
figured that the zend_compiler_globals dtor was the point of failure here.

TSRM needs to mark resources as 'done' following a free and doesn't in most
cases, but that's actually not the main problem we have in PHP-GTK (although
fixing it may well solve everybody else's, I'll check that on my travels).

Dmitry's "Optimized cleanup loops on request shutdown" commit(s) spanning
the 13th and 14th March broke Andrei's 'EG(full_tables_cleanup) = 1' ruse
(we needed user classes to be cleaned and internal classes to stay), but it
also broke TSRM shutdown completely for us. PHP-GTK classes are recognized
as being internal, but only the first one ever reaches the dtor - even when
there are no user-defined classes.

zend_hash_reverse_apply() is called at that point, so I'm guessing there's a
reentrancy issue behind my access violation crash.

I'll get back to this when I get some time, in the meantime if anyone
(Dmitry?) wants to play with this you need to know that I can't put the 5_2
compat stuff into CVS until Andrei OKs it, so PHP-GTK HEAD has to be built
against PHP_5_1 branch at present (anything after March 14th will do).

• Steph

Under CLI ZTS build, when loading a PHP-GTK 2 .dll built against anything
this side of PHP 5.1.2 release, I'm seeing an 'access violation' crash on
reaching:

 if (resource_types_table && !resource_types_table[j].done && 

resource_types_table[j].dtor) {
resource_types_table[j].dtor(p->storage[j], &p->storage);
}

Similar code is also used in:

tsrm_free_interpreter_context()
ts_free_thread()
ts_free_worker_threads()
ts_free_id()

I thought ts_free_thread() was crashing at that line too at first, but
that turned out to be because there's no 'done' check there, i.e. double
dtors are possible via some functions. It's still vaguely possible there's
a double flypast causing the crash I'm seeing, but I have everything I
know about protected now. (On my laptop that is.)

Eventually I found the resource id for PHP-GTK - the only extension I'm
loading during runtime, via php.ini - is 32nd out of a possible 32.
"Eventually", because that means I can't use ts_free_id() to avoid the
crash as advised by Frank (and Tony, and Dmitry, and anyone else who ever
used that MSHUTDOWN workaround for CLI). Interesting too, because the
resource that causes the crash appears to be something completely other.
Tracking down resource id #5 - and that's all I know about it apart from
it crashes - is a barrel of laughs, I'll let y'all know if I ever find out
which of the many possible extensions/files in ext/standard or core it is.

Short version: One line is problematic, and only in one function, and
presumably only under CLI SAPI. (CGI already doesn't call tsrm_shutdown(),
thanks to similar issues some 3 years ago).

The question is whether to take 'the Zeev approach' and simply comment out
the tsrm_shutdown() call at the end of sapi/cli/php_cli.c, or whether to
spend time knitting up a fine-grained approach to prevent the one bad line
in the function from being called in single-threaded environments? I think
I've given up on the idea of actually resolving the bug now...

Thoughts?

• oh, and don't make one of them 'chicken-and-egg' - this is definitively
  NOT related to the shutdown order in main.c!!!!!

• Steph

--

__________ NOD32 1.1380 (20060125) Information __________

This message was checked by NOD32 antivirus system.
http://www.eset.com

Best regards,
Marcus