Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » fd patch »

fd patch

php.internals

Zeev Suraski

23 years ago
Just a last heads up before the release here. Do we really want this patch in? I think that it was put in a bug-fix release in a rather hasty manner, with no real reason. We've already experienced one crash, there may be others - it's a very central piece of code. Weighing the pros and cons of this patch, it basically fixes a problem that isn't new or even unique to PHP, and existed ever since the days of PHP/FI 2. The symptoms of this problem arise fairly rarely, in situations where there's a combination of a web server that opens a very large number of files[*], and an operating system with a libc that can't handle fd's over 255 (Solaris). I think that saying that PHP is unusable under Solaris is a gross overstatement; I've been aware of this issue since 1996, and yet, I've come across less than a handful of real world cases that experienced this problem, versus hundreds of cases that worked flawlessly. By including the patch, we are fixing a problem that affects a very small percentage of the userbase on Solaris[*], at the price of a fairly big risk in breaking functionality for existing users on ALL operating systems. To summarize, including it in 4.3.2 (or anything in the 4.3.x series) doesn't sound like good math to me. v5/ZE2 already has a fix for this, and considering the urgency level of this issue, the sensitivity of the code it touches, and the amount of testing that 4.3.x will get before it gets released versus 5.x - waiting for 5 makes much more sense. For people who experience this specific issue, we can provide the patch for 4.3.x on php.net. I don't mean to start a big thread here - I feel very uncomfortable releasing such a sensitive patch with so little testing, and don't exactly understand why the problem that it addresses is suddenly so urgent. If there's consensus here that this patch is important for this 4.3.x release, so be it - I want to at least voice my opinion before it gets finalized. Zeev [*] Namely, users of multithreaded servers that actually use PHP in threaded mode, which are relatively few; And users of Apache, configured with hundreds of log files, which are also relatively few. There might be other examples, but the problem certainly does not affect a typical Apache/PHP setup. Specifically, it is NOT correlated in any way to the number of include/require files that you have in your application - as long as there's one fd under 255 handy, PHP will operate without any problems.

Sascha Schumann

23 years ago
On Tue, 20 May 2003, Zeev Suraski wrote:
> Just a last heads up before the release here. Do we really want this patch > in? I think that it was put in a bug-fix release in a rather hasty manner, > with no real reason. We've already experienced one crash, there may be > others - it's a very central piece of code.
Yes, it is indeed very central and that is the reason why the old code caused so much trouble for a long, long time. It fixes a nasty limitation on an important platform. Certain SAPI modules like NSAPI, AOLserver, Apache 2 and thttpd are severely affected by this issue - they are basically useless on Solaris. The patch has been extensively tested -- I have so much faith in it, that is already running at a customer's site on several Solaris installations. If you want more exposure for the patch, I suggest rolling another RC and using our far reaching announce mailing list for generating feedback regarding the RC's quality for once. - Sascha

Zeev Suraski

23 years ago
At 05:32 20/05/2003, Sascha Schumann wrote:
>On Tue, 20 May 2003, Zeev Suraski wrote: > > > Just a last heads up before the release here. Do we really want this patch > > in? I think that it was put in a bug-fix release in a rather hasty manner, > > with no real reason. We've already experienced one crash, there may be > > others - it's a very central piece of code. > > Yes, it is indeed very central and that is the reason why the > old code caused so much trouble for a long, long time.
It definitely wasn't too much trouble, there was next to no complaints about it.
> It > fixes a nasty limitation on an important platform. Certain > SAPI modules like NSAPI, AOLserver, Apache 2 and thttpd are > severely affected by this issue - they are basically useless > on Solaris.
Most of these SAPIs have been basically useless on all operating systems until recently. Most of them are still not considered stable for production use because of other threading issues. Despite this long list of SAPIs, Apache 1.x SAPI users far outnumber the users of these SAPIs. I had no doubt that you'd defend the introduction of this patch, you wouldn't have introduced it otherwise. I'm more interested in hearing what other people think, as I think they weren't made aware of the scope of the problem and the risks involved. Zeev

Sascha Schumann

23 years ago
Zeev, you are a bit late -- I'm afraid it would be riskier to remove the fd patch and all related changes at this point than just carrying on. I understand that you fear "late breakage", because you have been involved in such events a couple of times in the past. It's just natural to learn from such mistakes and try to prevent them in the future. In order to address that, I propose making a release candidate announcement to a wider audience. The GCC folks regularly use their announce mailing list for pre-releases. If we do the same, it would help us tremendously to put the spot on our release candidates and receive more diverse feedback. Any opinions? - Sascha

Zeev Suraski

23 years ago
At 06:08 20/05/2003, Sascha Schumann wrote:
> Zeev, > > you are a bit late -- I'm afraid it would be riskier to > remove the fd patch and all related changes at this point > than just carrying on.
Are you kidding? Rolling back to the rock solid code we've been using for the past few years will take approximately 5 minutes. I'll happily volunteer to do it, if that will be the decision, and if you're phobic for some reason to do it yourself. [pathetic attempt to draw fire snipped] Zeev

Sascha Schumann

23 years ago
Well, here is a plan which is far superior than solely relying on one's gut feelings. (a) Roll RC4 immediately* due to a number of new bug fixes. (b) Release a Pre-Announcement to announce@ and various dedicated PHP sites for maximum exposure. Emphasize that we want to hear about any new issues. (c) Evaluate data points within a week. (d) Make a decision after that. * I volunteer, if Jani is fed up. - Sascha

Shane Caraveo

23 years ago
If we're doing another RC, I'd like it to wait a couple days, I want to get another little fix in tomorrow night. I'd do it tonight, but left my osx off at work so cannot test it tonight. The patch will only effect fastcgi on non-windows, so it's relatively minor. Shane Sascha Schumann wrote:

Zeev Suraski

23 years ago
I'm kinda worried about those last-minute fastcgi (and CGI!). Last time we did that we had one of the biggest security holes in PHP's history. Especially regarding the fd patch, I think we need a bigger QA cycle, something that only comes around with a prolonged release cycle, as we'd have in v5 (where initially, people expect things to break anyway). The only thing that really doesn't add up for me is why Sascha considers this patch as a must-have for a bug-fix production release, when it has such limited implications, but such devastating potential. I just can't see the problem in putting up a patch for those who need it on php.net. Zeev At 06:54 20/05/2003, Shane Caraveo wrote:

Shane Caraveo

23 years ago
Zeev Suraski wrote:
> I'm kinda worried about those last-minute fastcgi (and CGI!). Last time > we did that we had one of the biggest security holes in PHP's history.
The fix in question is http://bugs.php.net/bug.php?id=23664, which leaves a feature completely broken. If this is possibly to be the last release prior to 5, I think it should go in. The security hole was always there, cleaning up other code allowed it to be hit, and had we ever had a test system in place to test PHP in the way it is used, we probably would have caught it. Anyway, it's no big deal to me if it doesn't go in. Shane

Derick Rethans

23 years ago
On Tue, 20 May 2003, Shane Caraveo wrote:
> Zeev Suraski wrote: > > I'm kinda worried about those last-minute fastcgi (and CGI!). Last time > > we did that we had one of the biggest security holes in PHP's history. > > The fix in question is http://bugs.php.net/bug.php?id=23664, which > leaves a feature completely broken. > If this is possibly to be the last release prior to 5, I think it should > go in.
We can always release 4.3.3 ... I don't think that will be a problem to put the other fixes in there. THe problem that I face is that 4.3.2 might be the most stable release ever, but that whole opinion chance with the merging of the FD patch. Shane, I think this can go safely into 4.3.3, why not start with that right after 4.3.2 is released? And perhaps only put the fd and FastCGI patch in there so that the release cycle can be short and not affect our most stable 4.3.2 release. regards, Derick
-- "my other box is your windows PC" ------------------------------------------------------------------------- Derick Rethans http://derickrethans.nl/ International PHP Magazine http://php-mag.net/ -------------------------------------------------------------------------

John Coggeshall

23 years ago
> as we'd have in v5 (where initially, people expect things to break anyway).
I think *we* expect things to break anyway, but talk to the user base and I think you'd find most of them expect the new flagship of PHP to do more than "break anyway". If your going to release PHP5 relying on the assumption that things are just going to break anyway so who cares, why bother with QA at all? John
-- -~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~- John Coggeshall john at coggeshall dot org http://www.coggeshall.org/ -~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~-

Zeev Suraski

23 years ago
At 10:46 20/05/2003, John Coggeshall wrote:
> > as we'd have in v5 (where initially, people expect things to break anyway). > >I think *we* expect things to break anyway, but talk to the user base >and I think you'd find most of them expect the new flagship of PHP to do >more than "break anyway". If your going to release PHP5 relying on the >assumption that things are just going to break anyway so who cares, why >bother with QA at all?
You missed the point. PHP 5 will go through betas for several months. It will be used by plenty of people who expect it to break before version 5.0.0 comes out. And if we keep ourselves realistic, most people will not be quick to move to 5.0.0 on their production servers, without first testing that everything works right. Many people will not move into a .0 release at all before at least a couple of bugfix releases are issued. All of that does not hold true for a bug-fix release such as 4.3.2. People are going to expect that upgrading from 4.3.x to 4.3.2 is going to be painless, and they're not going to put too many efforts in testing it. That's why it's so dangerous to put in a patch with such a far-reaching potential in a bug fix release, and why it's better to take advantage of the slower release cycle of v5. Zeev

Sascha Schumann

23 years ago
Get real Zeev. Even if there a minor issue or two with that patch, hell would not break loose. It would not be a catastrophe. Contrary, if we would stop innovating due to a single person's fears, PHP would lose a lot appeal as a viable platform. So, why don't you stop that hand waving and review/evaluate those changes to convince yourself of their maturity. That would be a far more constructive approach than prolonging this debate. - Sascha

Zeev Suraski

23 years ago
At 14:54 20/05/2003, Sascha Schumann wrote:
> Get real Zeev. Even if there a minor issue or two with that > patch, hell would not break loose. It would not be a > catastrophe. Contrary, if we would stop innovating due to > a single person's fears, PHP would lose a lot appeal as a > viable platform.
Since I promised not to turn this into a thread, I have no intention to accept your duel invitations. I'll repeat what I've said before - I didn't doubt for a second that you'd defend the inclusion of these patches (apparently, by all means necessary, for reasons I can only guess), as you introduced them. I'm interested in what others have to say, especially in light of your responses from the last few hours.
> So, why don't you stop that hand waving and review/evaluate > those changes to convince yourself of their maturity. That would > be a far more constructive approach than prolonging this debate.
I have. And I haven't found the crash bug Brian found. Neither did you. That's the kind of stuff you find through testing, because code analysis only goes so far. That's why there are certain thumb rules as to releasing software. Including such a fundamental change in a minor bug-fix release for the sake of a tiny fragment of the userbase, endangering the rest - goes against more than one of these rules. Zeev

George Schlossnagle

23 years ago
On Tuesday, May 20, 2003, at 08:18 AM, Zeev Suraski wrote:
> I'm interested in what others have to say, especially in light of > your responses from the last few hours.
With an invitation like that it's hard to resist.
> Including such a fundamental change in a minor bug-fix release for > the sake of a tiny fragment of the userbase, endangering the rest - > goes against more than one of these rules.
I agree completely with Zeev. This seems like something for php5 or 4.6.0 (or whatver branch is not currently taken as the text minor version. This seems like a big change for a sub-minor version change. George

Sterling Hughes

23 years ago
I just want to note I'm with Zeev here too. PHP 4.3.X are minor bugfix releases. This is pretty major, and imho too big a risk to take. Why don't we, as Zeev proposed, provide a patch for people who use Solaris on php.net? Its not to say that the new code is bad, its just too far reaching for a bugfix release, imho. There is always PHP 4.4 or PHP5, where the bug is already fixed. Until then, supplying a patch shouldn't be too terrible. -Sterling On Tue, 2003-05-20 at 10:02, George Schlossnagle wrote:
> On Tuesday, May 20, 2003, at 08:18 AM, Zeev Suraski wrote: > > > I'm interested in what others have to say, especially in light of > > your responses from the last few hours. > > With an invitation like that it's hard to resist. > > > Including such a fundamental change in a minor bug-fix release for > > the sake of a tiny fragment of the userbase, endangering the rest - > > goes against more than one of these rules. > > I agree completely with Zeev. This seems like something for php5 or > 4.6.0 (or whatver branch is not currently taken as the text minor > version. This seems like a big change for a sub-minor version change. > > George
-- "A business that makes nothing but money is a poor kind of business." - Henry Ford

Edin Kadribasic

23 years ago
On 20 May 2003, Sterling Hughes wrote:
> I just want to note I'm with Zeev here too. PHP 4.3.X are minor bugfix > releases. This is pretty major, and imho too big a risk to take. Why > don't we, as Zeev proposed, provide a patch for people who use Solaris > on php.net?
I disagree with that. PHP 4.3.x is going to be the latest released version of PHP for quite some time to come so it is my opinion that we have to be more flexible and allow not-so-minor changes. Otherwise you need to tell people that they can only expect improvements in the upcoming (and quite experimental) version of PHP that is due to be released in 6 to 12 months. Edin

Derick Rethans

23 years ago
On Tue, 20 May 2003, Edin Kadribasic wrote:
> On 20 May 2003, Sterling Hughes wrote: > > > I just want to note I'm with Zeev here too. PHP 4.3.X are minor bugfix > > releases. This is pretty major, and imho too big a risk to take. Why > > don't we, as Zeev proposed, provide a patch for people who use Solaris > > on php.net? > > I disagree with that. PHP 4.3.x is going to be the latest released version > of PHP for quite some time to come so it is my opinion that we have to be > more flexible and allow not-so-minor changes. Otherwise you need to tell > people that they can only expect improvements in the upcoming (and quite > experimental) version of PHP that is due to be released in 6 to 12 months.
We can always release 4.3.3 in a few weeks so that there is time to test this bugfix. Derick
-- "my other box is your windows PC" ------------------------------------------------------------------------- Derick Rethans http://derickrethans.nl/ International PHP Magazine http://php-mag.net/ -------------------------------------------------------------------------

Wez Furlong

23 years ago
I agree with Edin; the fd patch also fixes some problems under windows. George suggested that there is nothing to prevent 4.whatever that includes the fix; what I am suggesting is that there is no need to keep making all these releases within a short time of each other, and maintaining them (don't forget win32 build snapshots/maintenance!) when we are trying to focus on PHP5. *if* we do release further 4.x releases, they will be based on this same code branch anyway; why not hold off 4.3.2 just for a little while longer to QA this patch, then we can forget about PHP4 and move on with PHP5. It's getting quite difficult to maintain both the 4 and 5 code bases (from my perspective anyway) as there are a number of sufficiently new features in PHP5 that makes merging between the branches a painful manual process. Summary: Just QA the patch a little while longer, release it in 4.3.2 and move on. --Wez. On Tue, 20 May 2003, Edin Kadribasic wrote:

Zeev Suraski

23 years ago
At 17:13 20/05/2003, Edin Kadribasic wrote:
>On 20 May 2003, Sterling Hughes wrote: > > > I just want to note I'm with Zeev here too. PHP 4.3.X are minor bugfix > > releases. This is pretty major, and imho too big a risk to take. Why > > don't we, as Zeev proposed, provide a patch for people who use Solaris > > on php.net? > >I disagree with that. PHP 4.3.x is going to be the latest released version >of PHP for quite some time to come so it is my opinion that we have to be >more flexible and allow not-so-minor changes. Otherwise you need to tell >people that they can only expect improvements in the upcoming (and quite >experimental) version of PHP that is due to be released in 6 to 12 months.
I think that at the very least we should delay it to 4.3.3, or even push out a 4.4.0 release just for that, with fairly clear warnings. A dot-zero release like 4.4.0 will at least signal to people that it may introduce new bugs. The important part is that it shouldn't be in 4.3.2. Reasoning: [1] 4.3.0 was a pretty bad release, with lots of newly introduced bugs. [2] 4.3.2 has great potential for being a stable version of 4.3.0. [3] The fd patch endangers this. Since its introduction, at least 3 crash bugs have been found. It's not unrealistic that there'd be others. At the very least we should push out a version that's safe for everyone to upgrade to, a stable 4.3 version. If we decide to release a further version that includes this patch (which I personally think is a bad idea) so be it, but it should come with a clear "You don't need to upgrade unless you're experiencing this problem" message. This is something we can't say about 4.3.2. Zeev

Derick Rethans

23 years ago
On Tue, 20 May 2003, Zeev Suraski wrote:
> I don't mean to start a big thread here - I feel very uncomfortable > releasing such a sensitive patch with so little testing, and don't exactly > understand why the problem that it addresses is suddenly so urgent. If > there's consensus here that this patch is important for this 4.3.x release, > so be it - I want to at least voice my opinion before it gets finalized.
I have the same feelings, I was not really happy by putting it in in the PHP_4_3 either. (Though I'm not sure if I voiced that opinion on a list). I think it's up to our release guy (Jani) to decide what to do with it. Derick
-- "my other box is your windows PC" ------------------------------------------------------------------------- Derick Rethans http://derickrethans.nl/ International PHP Magazine http://php-mag.net/ -------------------------------------------------------------------------

Rasmus Lerdorf

23 years ago
On Tue, 20 May 2003, Derick Rethans wrote:
> On Tue, 20 May 2003, Zeev Suraski wrote: > > > I don't mean to start a big thread here - I feel very uncomfortable > > releasing such a sensitive patch with so little testing, and don't exactly > > understand why the problem that it addresses is suddenly so urgent. If > > there's consensus here that this patch is important for this 4.3.x release, > > so be it - I want to at least voice my opinion before it gets finalized. > > I have the same feelings, I was not really happy by putting it in in the > PHP_4_3 either. (Though I'm not sure if I voiced that opinion on a > list). I think it's up to our release guy (Jani) to decide what to do > with it.
My only reservation is that it went in somewhat late in the RC cycle. This change has been needed for quite a while. I know the issue itself has been around for quite a while, but threaded web servers have become much more common on Solaris in recent months and problems in our nsapi api have been worked out so PHP is now a viable iPlanet plugin. This was never really the case before. However, the last hurdle for decent iPlanet support is this patch. The second reason is performance. By moving away from stdio streams we save a stat() call for every single file open. Believe me, disk-touching syscalls is a killer. And finally, there has been wide support for testing this thing to death. We are definitely putting it through the ringer at Yahoo (hence the patch from Brian). It is running on php.net and Sascha has been pounding it as well. There may very well still be issues, but with a bit of dedicated testing, the performance benefits plus finally having real iPlanet support is worth a little extra pain as far as I am concerned. -Rasmus

Wez Furlong

23 years ago
I'm +1 for keeping the fd patch in the 4.3.2 release. --Wez. On Tue, 20 May 2003, Rasmus Lerdorf wrote: