Everything

php

Re: tsrm_shutdown() and the CLI SAPI

php.internals

Frank M. Kromann

20 years ago

tsrm_shutdown() is already comented out in the CGI version, most likely as
a fix to the same kind of problem there. Perhaps enabling that again will
show the same kind of problems?

- Frank

> At the moment I'm seeing it with Tidy under 5_2.  Just about to get Tony

to

> check.
>
> ----- Original Message -----
> From: "Dmitry Stogov" <dmitry@zend.com>
> To: "'Steph Fox'" <steph@zend.com>; "'Xuefer'" <xuefer@gmail.com>;

"'Andi

> Gutmans'" <andi@zend.com>
> Cc: "'internals'" <internals@lists.php.net>; "'Antony Dovgal'"
> <antony@zend.com>
> Sent: Wednesday, May 31, 2006 2:55 PM
> Subject: RE: [PHP-DEV] tsrm_shutdown() and the CLI SAPI
>
>
> > Steph,
> >
> > Commenting tsrm_shutdown() or FreeLibrary() is just hidding real

bugs.

> > Of course this is a solution, but not excellent.
> >
> > Right now I haven't enough time even for ZE and PHP themself, so I

don't

> > like spent it looking into php-gtk on windows.
> > (I don't know GTK at all).
> >
> > Do you know any other extension from PHP distribution that cause the

same

> > crash?
> > Could you provide instruction how to reproduce it?
> >
> > Thanks. Dmitry.
> >
> >> -----Original Message-----
> >> From: Steph Fox [mailto:steph@zend.com]
> >> Sent: Wednesday, May 31, 2006 3:28 PM
> >> To: Dmitry Stogov; 'Xuefer'; 'Andi Gutmans'
> >> Cc: 'internals'; Antony Dovgal
> >> Subject: Re: [PHP-DEV] tsrm_shutdown() and the CLI SAPI
> >>
> >>
> >> > Oh well, now I see the difference.
> >> > With my patch PHP starts call FreeLibrary() for dll extension. I

am

> >> > wonder why it wasn't called before. :)
> >>
> >> Pure luck, by the look of it :)
> >>
> >> > Probably some other bug that is the reason of the crash was
> >> masked by
> >> > omitting FreeLibrary(). I know Tony fixed some kind of such bugs

in

Steph

20 years ago

Hi Frank,

> tsrm_shutdown() is already comented out in the CGI version, most likely as
> a fix to the same kind of problem there. Perhaps enabling that again will
> show the same kind of problems?

Yes, it would, given the root cause - but would you really want to break the
whole of PHP for an academic exercise?

The more interesting thing is that none of this happens under CLI debug.
I've still no clue why.

I also think I've somehow corrupted something here with my debug lines, I'm
crashing 'differently' now <sigh />.

- Steph

Andi Gutmans

20 years ago

At 08:23 AM 5/31/2006, Steph Fox wrote:

>Hi Frank,
>
>>tsrm_shutdown() is already comented out in the CGI version, most likely as
>>a fix to the same kind of problem there. Perhaps enabling that again will
>>show the same kind of problems?
>
>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.
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.

Andi

Steph

20 years ago

>>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 Gutmans

20 years ago

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:

Dmitry Stogov

20 years ago

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.

Steph

20 years ago

Wrong crash.  Mine doesn't happen in debug mode.

----- Original Message -----
From: "Dmitry Stogov" <dmitry@zend.com>
To: "'Andi Gutmans'" <andi@zend.com>; "'Steph Fox'" <steph@zend.com>;
"'Frank M. Kromann'" <frank@kromann.info>
Cc: "'internals'" <internals@lists.php.net>; "'Antony Dovgal'"
<antony@zend.com>; "'Xuefer'" <xuefer@gmail.com>
Sent: Thursday, June 01, 2006 1:05 PM
Subject: RE: [PHP-DEV] tsrm_shutdown() and the CLI SAPI

Andi Gutmans

20 years ago

I don't understand this patch. Doesn't this completely remove
printing memory leaks? We never fixed this bug but as it was only
relevant to the debug version and dl() we were OK with it... Of
course there's also the option of not calling DL_UNLOAD().

Andi

At 04:05 AM 6/1/2006, Dmitry Stogov wrote:

Steph

20 years ago

Hi Andi,

>I don't understand this patch. Doesn't this completely remove printing
>memory leaks? We never fixed this bug but as it was only relevant to the
>debug version and dl() we were OK with it... Of course there's also the
>option of not calling DL_UNLOAD().

Things got a bit crossed today :-\

This patch fixes a completely different bug that Dmitry came across on his
travels.

- Steph

Dmitry Stogov

20 years ago

Hi Steph,

I finally found the reason of crash.
It has nothing related to ZE bugs.

You build php-gtk with "/MT" option.

This is completely wrong, because you use two different libc and two
different heaps.
As result when ZE calls FreeLibrary(), one of heaps is destroyed and all
pointers that were allocated from inside php-gtk.dll become inaccessible.

Rebuilding php-gtk with "/MD" fixes the problem.

Thanks. Dmitry.

Steph

20 years ago

Hi Dmitry,

Hm yeah that forces it to use libcmt... that'd hurt :)

Thank you for finding that! I'd looked through the debug build stuff (which
is correct) when trying to find out why PHP-GTK didn't crash at the same
point as Tidy - but I didn't check the release flags while I was there.

Still, I wouldn't say it was a waste of time - we did at least find three
other bugs through it, two of which are fixed already and one of which has
two proposed fixes waiting for evaluation. So I guess the arguments for
keeping DL_UNLOAD alive and well are proven good.

- Steph

ps Andrei - we're on for the weekend!

Dmitry Stogov

20 years ago

This patch fixes very specific bug.

"php -v" calls RINIT() functions, php with php-gtk throws error from RINIT
if we run php from telent/ssh session, because it cannot open DISPLAY. As
result we don't go into request shutdown (shutdown_memory_manager() is
called there) sequence and doing module shutdown and tsrm_shutdown().
Because request_shutdown() wasn't done we didn't cleared emalloced memory,
and trying to do it during tsrm_shutdown() but some globals (sapi globals)
are destroyed in this time. As result we get a crash.

Thanks. Dmitry.

Steph

20 years ago

We were hoping to release ours...

----- Original Message -----
From: "Andi Gutmans" <andi@zend.com>
To: "Steph Fox" <steph@zend.com>; "Frank M. Kromann" <frank@kromann.info>
Cc: "'internals'" <internals@lists.php.net>; "'Antony Dovgal'"
<antony@zend.com>; "Dmitry Stogov" <dmitry@zend.com>; "'Xuefer'"
<xuefer@gmail.com>
Sent: Thursday, June 01, 2006 8:28 AM
Subject: Re: [PHP-DEV] tsrm_shutdown() and the CLI SAPI