Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:14819
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <5.1.0.14.2.20050210184653.01d461b0@localhost>
Date: Thu, 10 Feb 2005 18:52:32 -0800
To: Jani Taskinen <sniper@iki.fi>
Cc: internals@lists.php.net
In-Reply-To: <Pine.LNX.4.61.0502110416120.31271@arfg.argcubovn.sv>
References: <5.1.0.14.2.20050210150822.01d3f6e0@localhost>
	<5.1.0.14.2.20050210150822.01d3f6e0@localhost>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Subject: Re: [PHP-DEV] Re: [ZEND-ENGINE-CVS] cvs: TSRM(PHP_4_3) /
	TSRM.c TSRM.h tsrm_config.nw.h tsrm_config_common.h tsrm_nw.c
	tsrm_nw.h tsrm_virtual_cwd.c tsrm_virtual_cwd.h tsrm_win32.c
	tsrm_win32.h  
From: andi@zend.com (Andi Gutmans)

At 04:45 AM 2/11/2005 +0200, Jani Taskinen wrote:
>On Thu, 10 Feb 2005, Andi Gutmans wrote:
>
>>Jani,
>>
>>This looks like a very serious change for 4.3.x series. It should really 
>>be discussed on internals@ before you fire away.
>
>     Sure, let's discuss. This is the only way to get any attention to
>     anything anyway. Just to note first of all that I asked Ilia (who was 
> the release
>     god for PHP_4_3 last time I checked..) and he didn't object to 
> syncing HEAD
>     TSRM as long as it's fixes only.

Jani,
Recently you are commiting too many patches which you personally haven't 
reviewed/understood just to get attention. You should realize many of us 
are busy and can't have immediate response to every single issue that comes 
up. I do try and be responsive when I'm emailed with a specifc problem that 
requires attention but even that can take a week or so. I think it's not 
too much to ask and it's no excuse to just commit bad patches because they 
aren't being attended to. Better not to commit if you're not 100% sure than 
to commit bad stuff which in the good case needs to be reverted and in the 
worse case, no one notices and makes it in. You know exactly what I'm 
talking about and how this has been happening quite often.

>     And it does fix a lot: There are no more leaks in ZTS mode.
>     Plus it's actually maintainable now when all those whitespace 
> changes, etc.
>     are in the PHP_4_3 branch too.

I remember there were specific reasons why this wasn't commited to 4.3.x 
because in the past we had segfaults and we said we'll hunt them down in 
5.1. BTW, the leaks only affect multi-threaded servers where threads die 
and respawn (very rare in our context).


>>There was also agreement a while ago that realpath caching will only be 
>>in 5.1.x so that it gets enough testing.
>
>     AFAICT, it's not enabled in HEAD nor it is enabled in PHP_4_3 so I don't
>     really see any problems there.

Last time I checked it was enabled by default.

I don't feel like arguing. I just wish you consult instead of commiting 
just to get what you consider "the proper" attention. It's not always easy 
to provide 0 response time for all issues and especially not for something 
like this which is VERY far from being critical, so that Windows users can 
run unstable ISAPI in multi-threaded mode without leaking.... Well it'll 
probably crash...

Andi


>>Also as most of these fixes are Netware/Windows related, I don't think 
>>it's worth the risk of crashes and other problems we said we might have 
>>with some of the changes.
>
>     Which change are you referring to here? (And I wouldn't worry about 
> netware here..)
>
>     --Jani
>