Everything

php

[PATCH] Deprecate use of stdio

php.internals

Sascha Schumann

23 years ago

The following patches are aimed at deprecating the use of
stdio by the script lexer in PHP 4, because certain stdio
libraries impose restrictive limits on the number of open
handles*.

The patches have been split functionally for the benefit of
the reviewer.

Patch 1 adds a stream interface which fills in a
zend_file_handle.  This is simply a modified copy of the
open_as_file function.

Patch 2 adds zend_open() which expects the implementation to
fill in a zend_file_handle.

Patch 3 makes PHP supply a zend_open implementation.

Patch 4 modifies the lexer skeleton so that it can read using
POSIX functions and stdio.  It also makes
open_file_for_scanning use zend_open.

* Background: Even the most modern Solaris ships with a stdio
library which cannot handle fds over 255.  This severely
limits any web server which may open more than 255 fds per
process, because it won't be able to parse PHP scripts
anymore.

I've evaluated integrating sfio into the build process, but
as it turns out, a full and complete integration would be a
lot more complex than addressing the issue up-front.

- Sascha

Wez Furlong

23 years ago

+1 from me, but we need to make an extra effort to stress test the
lexer (the ini file parser as well as the language parser) in our QA
process.

Also, for the sake of binary compatibility (not sure just how much of an
issue this is here), its "nicer" to add the zend_open function pointer
to the end of the utility functions structure, rather than in the
middle.

What do Zeev/Andi think of this?

--Wez.

On Fri, 2 May 2003, Sascha Schumann wrote:

Sascha Schumann

23 years ago

On Fri, 2 May 2003, Wez Furlong wrote:

> +1 from me, but we need to make an extra effort to stress test the
> lexer (the ini file parser as well as the language parser) in our QA
> process.

In our what?

A patched PHP inside of thttpd does not show any anomalies
after 100K ab requests:

test.php:

<?php

require "bar";
include "foo";
include "notthere";

bar:

<?echo "hi";

foo:

<?phpinfo();

As I mentioned earlier, including from URLs appears to be
broken even without my patches (we could call that a security
feature).

> Also, for the sake of binary compatibility (not sure just how much of an
> issue this is here), its "nicer" to add the zend_open function pointer
> to the end of the utility functions structure, rather than in the
> middle.

Moved.  It would only affect applications which statically
initialize the structure.

- Sascha

Rasmus Lerdorf

23 years ago

Something for 4.3.3 or do you fear the stability of these changes?  I
would consider the fact that PHP barely works on Solaris a rather large
bug and have no problem seeing this as a bug fix.  My ulterior motive is
of course that we lose the stupid stdio buffer-size stat() call which I
could sorely use in the current 4.3 tree.

-Rasmus

On Fri, 2 May 2003, Sascha Schumann wrote:

Wez Furlong

23 years ago

I'm happy to see this 4.3.2 also; the stability should be pretty good;
AFAICT, Sascha took the middle ground between my ZE2 patch and the
current ZE1 lexer code, so the changes aren't particularly drastic.

We just need to emphasize that the QA team tell their friends to tell
their friends to test it hard when we release the next RC :)

--Wez.

On Fri, 2 May 2003, Rasmus Lerdorf wrote:

Rasmus Lerdorf

23 years ago

On Fri, 2 May 2003, Wez Furlong wrote:

> I'm happy to see this 4.3.2 also; the stability should be pretty good;
> AFAICT, Sascha took the middle ground between my ZE2 patch and the
> current ZE1 lexer code, so the changes aren't particularly drastic.
>
> We just need to emphasize that the QA team tell their friends to tell
> their friends to test it hard when we release the next RC :)

I said 4.3.3 actually.  4.3.2 has been lingering for way too long and I'd
like to see that get out there soon.

-Rasmus

Jani Taskinen

23 years ago

On Fri, 2 May 2003, Rasmus Lerdorf wrote:

>On Fri, 2 May 2003, Wez Furlong wrote:
>> I'm happy to see this 4.3.2 also; the stability should be pretty good;
>> AFAICT, Sascha took the middle ground between my ZE2 patch and the
>> current ZE1 lexer code, so the changes aren't particularly drastic.
>>
>> We just need to emphasize that the QA team tell their friends to tell
>> their friends to test it hard when we release the next RC :)
>
>I said 4.3.3 actually.  4.3.2 has been lingering for way too long and I'd
>like to see that get out there soon.

I was about to release it tomorrow, but then I see that
some showstopper has crept in:

http://bugs.php.net/bug.php?id=23454

Possibly side-effect of fixing an other bug.
I've no idea and no time to investigate that right now.

--Jani

Rasmus Lerdorf

23 years ago

On Sun, 4 May 2003, Jani Taskinen wrote:

>     I was about to release it tomorrow, but then I see that
>     some showstopper has crept in:
>
>       http://bugs.php.net/bug.php?id=23454
>
>     Possibly side-effect of fixing an other bug.
>     I've no idea and no time to investigate that right now.

I'll take a look

Rasmus Lerdorf

23 years ago

>     I was about to release it tomorrow, but then I see that
>     some showstopper has crept in:
>
>       http://bugs.php.net/bug.php?id=23454
>
>     Possibly side-effect of fixing an other bug.
>     I've no idea and no time to investigate that right now.

Ilia's fix for bug #20796

http://cvs.php.net/diff.php/php4/main/php_variables.c?login=2&r1=1.45&r2=1.46&ty=u

Broke this.  He switched the two symbol tables in
php_register_variable_ex() so that the autoglobal array is populated first
but then the symboltable2 zend_hash_update() overwrites the previous one
so only the last array element appears.  Reverting this patch fixes 23454
but brings back 20796.

-Rasmus

Jani Taskinen

23 years ago

On Sun, 4 May 2003, Rasmus Lerdorf wrote:

>>     I was about to release it tomorrow, but then I see that
>>     some showstopper has crept in:
>>
>>       http://bugs.php.net/bug.php?id=23454
>>
>>     Possibly side-effect of fixing an other bug.
>>     I've no idea and no time to investigate that right now.
>
>Ilia's fix for bug #20796
>
>http://cvs.php.net/diff.php/php4/main/php_variables.c?login=2&r1=1.45&r2=1.46&ty=u
>
>Broke this.  He switched the two symbol tables in
>php_register_variable_ex() so that the autoglobal array is populated first
>but then the symboltable2 zend_hash_update() overwrites the previous one
>so only the last array element appears.  Reverting this patch fixes 23454
>but brings back 20796.

Yeah..I'd say we don't revert it, it's been like this since 4.3.0.

--Jani

Derick Rethans

23 years ago

On Sun, 4 May 2003, Jani Taskinen wrote:

> >
> >Broke this.  He switched the two symbol tables in
> >php_register_variable_ex() so that the autoglobal array is populated first
> >but then the symboltable2 zend_hash_update() overwrites the previous one
> >so only the last array element appears.  Reverting this patch fixes 23454
> >but brings back 20796.
>
>     Yeah..I'd say we don't revert it, it's been like this since 4.3.0.

I agree with that, people can always turn off register_globals (as
they're using the super globals anyway there is no real need to have
them on anyway).

Derick

--
"my other box is your windows PC"
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
PHP Magazine - PHP Magazine for Professionals       http://php-mag.net/
-------------------------------------------------------------------------

Sascha Schumann

23 years ago

On Fri, 2 May 2003, Wez Furlong wrote:

> I'm happy to see this 4.3.2 also; the stability should be pretty good;
> AFAICT, Sascha took the middle ground between my ZE2 patch and the
> current ZE1 lexer code, so the changes aren't particularly drastic.

Btw, your ZE2 modifications deactivate the sole code which
uses yy_interactive (YY_INPUT).  Was there any particular
reason why you did not simply nuke the yy_interactive stuff?

- Sascha

Wez Furlong

23 years ago

For the sake of the interactive-mode of CLI PHP.
However, it seemed to be broken anyway (prior to my patch), so the
status of this for ZE2 is "pending".

--Wez.


On Fri, 2 May 2003, Sascha Schumann wrote:

Sascha Schumann

23 years ago

On Fri, 2 May 2003, Rasmus Lerdorf wrote:

> Something for 4.3.3 or do you fear the stability of these changes?  I
> would consider the fact that PHP barely works on Solaris a rather large
> bug and have no problem seeing this as a bug fix.

At this point in time, I'm confident that the patch[1] works
reliably on all Unixes which are supported by the current PHP
4.3 tree.  Regressions are not to be expected.

Win32 is an open question -- I think I remember something
along the line that stdio works better on Win32, but that
should be answered by a local expert.  If there is such a
preference, it can be simply added to main/streams.c.

> My ulterior motive is
> of course that we lose the stupid stdio buffer-size stat() call which I
> could sorely use in the current 4.3 tree.

Avoiding regressions is my goal.  Optimizations can be done
in a second step.

[1] http://www.php.net/~sas/patch-lexer-no-stdio

- Sascha

Sascha Schumann

23 years ago

Here is an updated version.  Patch 01/03 are unchanged.

- Fixed fread argument order so that even my glibc likes it
- Improved file_handle code in the ini scanner
- Moved open_function struct member to the end

- Sascha

J Smith

23 years ago

I've been playing with sfio on Solaris quite a bit recently and I've had our
site working on iPlanet with sfio for a couple of weeks. Seems to be doing
okay so far with something between 4.3.1 and 4.3.2. Getting sfio to link
was a bit of pain in the arse at first, but it came down to a couple of
simple LIBS arguments to get it going.

As you say, fixing the problem at the root would be preferable, but sfio
might be a decent intermediary step.

I can test the patches and such on Solaris 8 SPARC, and if you (or anybody
who needs it, within reason of course) would like access to a Solaris
shell, I can arrange it. (It's slightly limited by a 300 MB quota, but
that's pretty much it.)

J


Sascha Schumann wrote:

Wez Furlong

23 years ago

Hi Jay,

This patch solves a lot of problems within PHP itself, but it is still a
good thing (tm) to link against sfio under Solaris in any case (third
party libs, and some of the less popular/maintained extensions will
still be using stdio).

So, the patch on its own is good, the patch combined with sfio is even
better. (Why can't Solaris fix their damned libc??).

If you can test it under iPlanet (initially, without using sfio), the
feedback would be much appreciated.

--Wez.

On Fri, 2 May 2003, Jay Smith wrote:

Sterling Hughes

23 years ago

Just a note (didn't have time to review the patch, but I trust you :-),
that I'm +1 on the idea.

-Sterling

On Thu, 2003-05-01 at 19:04, Sascha Schumann wrote:

>     The following patches are aimed at deprecating the use of
>     stdio by the script lexer in PHP 4, because certain stdio
>     libraries impose restrictive limits on the number of open
>     handles*.
>
>     The patches have been split functionally for the benefit of
>     the reviewer.
>
>     Patch 1 adds a stream interface which fills in a
>     zend_file_handle.  This is simply a modified copy of the
>     open_as_file function.
>
>     Patch 2 adds zend_open() which expects the implementation to
>     fill in a zend_file_handle.
>
>     Patch 3 makes PHP supply a zend_open implementation.
>
>     Patch 4 modifies the lexer skeleton so that it can read using
>     POSIX functions and stdio.  It also makes
>     open_file_for_scanning use zend_open.
>
>     * Background: Even the most modern Solaris ships with a stdio
>       library which cannot handle fds over 255.  This severely
>       limits any web server which may open more than 255 fds per
>       process, because it won't be able to parse PHP scripts
>       anymore.
>
>       I've evaluated integrating sfio into the build process, but
>       as it turns out, a full and complete integration would be a
>       lot more complex than addressing the issue up-front.
>
>     - Sascha
>
> ______________________________________________________________________
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

--
"First they ignore you, then they laugh at you,
then they fight you, then you win."
- Gandhi