Re: [PHP-DEV] destructors and output_buffering
Hi! On Tue, Sep 2, 2014 at 12:56 AM, Stas Malyshev wrote: > Hi! > > I've created a proposed fix for #67644: > > https://github.com/php/php-src/pull/798 Looks like this fix breaks some tests, so I'm not feeling good putting it into 5.4. I'll revert it for now and try to figure out a better solution later.
Re: [PHP-DEV] destructors and output_buffering
Hi Stas, Actually I would love flushing ob_start() in my own shutdown function, but since my application is a library called in auto_prepend_file, I have to protect ob_start() from being flushed too early by some wrong code in the client application. Hence I'm removing the PHP_OUTPUT_HANDLER_CLEANABLE, PHP_OUTPUT_HANDLER_FLUSHABLE & PHP_OUTPUT_HANDLER_REMOVABLE, but then I'm not able myself to flush the buffer :( A great "new" feature would be to optionally give a name to a buffer, and then allow to flush this buffer only if its name is passed through ob_end_clean / ob_end_flush / ... Thanks, Jocelyn Le 02/09/2014 21:29, Stas Malyshev a écrit : Hi! One line fix, nice :) A quick question about how resources are freed : if some variables are computed inside a singleton and read through a myclass::instance()->get_my_variable(), should we expect to have those variables available when calling ob_start() callbacks ? From experiment, they are still available, but since zend_call_destructors is called before ob_start(), I wonder if this is a safe behaviour. Generally, it would depend on when they were created. If they were created before shutdown stage and you try to access them in OB shutdown stage, it may be too late and the dtor may have been called for them already. Doing too much stuff on shutdown is not really a good idea, as you can not rely on resources being there. If you really need to do something on shutdown, I'd advise register_shutdown_function() as it is called first, before anything is really shut down. Relying on OB handlers shutdown is not a good idea as by then at least part of the shutdown happened. If you do something with OB handlers, I'd propose to add a shutdown function that flushes the OB, so that you are sure everything is wrapped up while the engine is still in predictable state and not half-shutdown. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
Hi! > One line fix, nice :) > > A quick question about how resources are freed : > if some variables are computed inside a singleton and read through a > myclass::instance()->get_my_variable(), should we expect to have those > variables available when calling ob_start() callbacks ? > > From experiment, they are still available, but since > zend_call_destructors is called before ob_start(), I wonder if this is a > safe behaviour. Generally, it would depend on when they were created. If they were created before shutdown stage and you try to access them in OB shutdown stage, it may be too late and the dtor may have been called for them already. Doing too much stuff on shutdown is not really a good idea, as you can not rely on resources being there. If you really need to do something on shutdown, I'd advise register_shutdown_function() as it is called first, before anything is really shut down. Relying on OB handlers shutdown is not a good idea as by then at least part of the shutdown happened. If you do something with OB handlers, I'd propose to add a shutdown function that flushes the OB, so that you are sure everything is wrapped up while the engine is still in predictable state and not half-shutdown. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
Hi, One line fix, nice :) A quick question about how resources are freed : if some variables are computed inside a singleton and read through a myclass::instance()->get_my_variable(), should we expect to have those variables available when calling ob_start() callbacks ? From experiment, they are still available, but since zend_call_destructors is called before ob_start(), I wonder if this is a safe behaviour. Thanks, Jocelyn Le 02/09/2014 09:56, Stas Malyshev a écrit : Hi! I've created a proposed fix for #67644: https://github.com/php/php-src/pull/798 It is the most conservative one which I'm thinking of putting into 5.4, so that at least we won't have segfaults. If we want to improve upon it, we can do that in 5.5/5.6, but I'd like at least plug the segfaults in 5.4 for now. Any objections to this or better proposals? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
Hi! I've created a proposed fix for #67644: https://github.com/php/php-src/pull/798 It is the most conservative one which I'm thinking of putting into 5.4, so that at least we won't have segfaults. If we want to improve upon it, we can do that in 5.5/5.6, but I'd like at least plug the segfaults in 5.4 for now. Any objections to this or better proposals? -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
Hi, Another possible behaviour would be to trigger a fatal when trying to access a partially destroyed object in ob_start. It took me a while to actually figure out why the memory was corrupted when doing stuff in ob_start() (even before playing with the AMQP lib). Having a fatal would allow me to save a lot of time, and it should not introduce any BC issue since any existing code accessing a partially destroyed object could randomly crash at some point. Jocelyn Le 31/08/2014 20:48, Stas Malyshev a écrit : Hi! Instead of iterating through all objects and setting a flag, can't we set a global flag that object dtors are not called after this point? We could, but that would probably break some code that expects dtors to be actually called. E.g. in the bug, the library there seems to do a lot from the dtor, and if we just shut it down it may break the library. I don't think we can do this in 5.x. And I don't think we could get rid of the object flag in that case, since there could be cases of circular links that may still require this flag to avoid double calling of dtor (even not on shutdown). This both solves the issue of new objects being created after the fact and makes shutdown less expensive. I'm not sure expense of the shutdown is that big a deal though. Bigger deal is avoiding crashes on the shutdown. It seems to be that disabling dtors on all objects there is a bit too harsh - after all, for most of them it is fine, it's just those that linger after the OB code run _and_ have dependencies on other objects that create trouble. Another dtor round, for example, should resolve it, or just marking only those that linger as destroyed. This seems to have less impact that just banning dtors altogether. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
Hi! > Instead of iterating through all objects and setting a flag, can't we > set a global flag that object dtors are not called after this point? We could, but that would probably break some code that expects dtors to be actually called. E.g. in the bug, the library there seems to do a lot from the dtor, and if we just shut it down it may break the library. I don't think we can do this in 5.x. And I don't think we could get rid of the object flag in that case, since there could be cases of circular links that may still require this flag to avoid double calling of dtor (even not on shutdown). > This both solves the issue of new objects being created after the fact > and makes shutdown less expensive. I'm not sure expense of the shutdown is that big a deal though. Bigger deal is avoiding crashes on the shutdown. It seems to be that disabling dtors on all objects there is a bit too harsh - after all, for most of them it is fine, it's just those that linger after the OB code run _and_ have dependencies on other objects that create trouble. Another dtor round, for example, should resolve it, or just marking only those that linger as destroyed. This seems to have less impact that just banning dtors altogether. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
On Sun, Aug 31, 2014 at 3:33 AM, Stas Malyshev
wrote:
> Hi!
>
> I was looking at bug https://bugs.php.net/bug.php?id=67644 and it looks
> like we have a bit of a problem with output buffering and dtors on
> shutdown. Basically, right now our code looks like this:
>
> /* 2. Call all possible __destruct() functions */
> zend_try {
> zend_call_destructors(TSRMLS_C);
> } zend_end_try();
>
> /* 3. Flush all output buffers */
> zend_try {
>
> // And here we go flushing output buffers
>
> Now, since ob functions can have userland handlers, these handlers can
> create new objects. And these objects will not benefit from orderly dtor
> round that zend_call_destructors(TSRMLS_C) is providing - instead their
> dtors will be called from zend_objects_store_free_object_storage() and
> by then their environment may already be ruined and their dtors can not
> run properly.
>
> OTOH, moving __destruct after output buffers may not be good either, as
> dtors may output something.
>
> The problem seems to be resolved if I either duplicate
> zend_call_destructors(TSRMLS_C); after the output buffers flushing or put
> zend_objects_store_mark_destructed(&EG(objects_store) TSRMLS_CC);
> there.
>
> Of course, the second solution has the downside of not calling the dtors
> of objects that were created while flushing ob buffers. The
> zend_call_destructors would work but since output buffering is supposed
> to be shut down by then, I wonder if it won't also have bad consequences
> if these dtors do something with output buffering.
>
> I'm leaning towards the second solution - if you create the objects so
> late on shutdown stage, you shouldn't really expect them to be destroyed
> normally, otherwise the cycle would never end. However, we could
> consider the first option too. Any thoughts?
>
Instead of iterating through all objects and setting a flag, can't we set a
global flag that object dtors are not called after this point? This both
solves the issue of new objects being created after the fact and makes
shutdown less expensive.
Nikita
Re: [PHP-DEV] destructors and output_buffering
On 31 Aug, 2014, at 12:40 pm, Stas Malyshev wrote: > Hi! > >> This is just a thought; could we delay the call to >> `zend_call_destructors` ONLY IF there’s output buffering taking place >> (i.e. ob_get_level() > 0)? > > That wouldn't help - imagine this: > 1. ob_start is set > 2. shutdown is starting > 3. ob functions shut down, call function foo > 4. function foo creates an object of class FooBar > 5. ob shutdown ends, all output is flushed, etc. > 6. FooBar::__destruct is run and tries to output something So let it output something ... Trying to output something in a destructor after flushing the output seems rather fishy; at the same time I’m quite aware that it’s impossible to predict what some developers would expect to happen in such cases. > > That scenario still may have a problem. I'll check more into if it's > really a big deal outputting after OB shutdown (after all, some other > things may lead to it too) but conditioning dtor move does not solve > this problem. If it works this way we may as well move them there > permanently. Yeah, I can see now how my suggestion doesn’t really need the condition then :) > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
Hi! > This is just a thought; could we delay the call to > `zend_call_destructors` ONLY IF there’s output buffering taking place > (i.e. ob_get_level() > 0)? That wouldn't help - imagine this: 1. ob_start is set 2. shutdown is starting 3. ob functions shut down, call function foo 4. function foo creates an object of class FooBar 5. ob shutdown ends, all output is flushed, etc. 6. FooBar::__destruct is run and tries to output something That scenario still may have a problem. I'll check more into if it's really a big deal outputting after OB shutdown (after all, some other things may lead to it too) but conditioning dtor move does not solve this problem. If it works this way we may as well move them there permanently. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] destructors and output_buffering
On 31 Aug, 2014, at 9:33 am, Stas Malyshev wrote:
> Hi!
>
> I was looking at bug https://bugs.php.net/bug.php?id=67644 and it looks
> like we have a bit of a problem with output buffering and dtors on
> shutdown. Basically, right now our code looks like this:
>
> /* 2. Call all possible __destruct() functions */
> zend_try {
> zend_call_destructors(TSRMLS_C);
> } zend_end_try();
>
> /* 3. Flush all output buffers */
> zend_try {
>
> // And here we go flushing output buffers
>
> Now, since ob functions can have userland handlers, these handlers can
> create new objects. And these objects will not benefit from orderly dtor
> round that zend_call_destructors(TSRMLS_C) is providing - instead their
> dtors will be called from zend_objects_store_free_object_storage() and
> by then their environment may already be ruined and their dtors can not
> run properly.
>
> OTOH, moving __destruct after output buffers may not be good either, as
> dtors may output something.
>
> The problem seems to be resolved if I either duplicate
> zend_call_destructors(TSRMLS_C); after the output buffers flushing or put
> zend_objects_store_mark_destructed(&EG(objects_store) TSRMLS_CC);
> there.
>
> Of course, the second solution has the downside of not calling the dtors
> of objects that were created while flushing ob buffers. The
> zend_call_destructors would work but since output buffering is supposed
> to be shut down by then, I wonder if it won't also have bad consequences
> if these dtors do something with output buffering.
>
> I'm leaning towards the second solution - if you create the objects so
> late on shutdown stage, you shouldn't really expect them to be destroyed
> normally, otherwise the cycle would never end. However, we could
> consider the first option too. Any thoughts?
This is just a thought; could we delay the call to `zend_call_destructors` ONLY
IF there’s output buffering taking place (i.e. ob_get_level() > 0)?
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] destructors and output_buffering
Hi!
I was looking at bug https://bugs.php.net/bug.php?id=67644 and it looks
like we have a bit of a problem with output buffering and dtors on
shutdown. Basically, right now our code looks like this:
/* 2. Call all possible __destruct() functions */
zend_try {
zend_call_destructors(TSRMLS_C);
} zend_end_try();
/* 3. Flush all output buffers */
zend_try {
// And here we go flushing output buffers
Now, since ob functions can have userland handlers, these handlers can
create new objects. And these objects will not benefit from orderly dtor
round that zend_call_destructors(TSRMLS_C) is providing - instead their
dtors will be called from zend_objects_store_free_object_storage() and
by then their environment may already be ruined and their dtors can not
run properly.
OTOH, moving __destruct after output buffers may not be good either, as
dtors may output something.
The problem seems to be resolved if I either duplicate
zend_call_destructors(TSRMLS_C); after the output buffers flushing or put
zend_objects_store_mark_destructed(&EG(objects_store) TSRMLS_CC);
there.
Of course, the second solution has the downside of not calling the dtors
of objects that were created while flushing ob buffers. The
zend_call_destructors would work but since output buffering is supposed
to be shut down by then, I wonder if it won't also have bad consequences
if these dtors do something with output buffering.
I'm leaning towards the second solution - if you create the objects so
late on shutdown stage, you shouldn't really expect them to be destroyed
normally, otherwise the cycle would never end. However, we could
consider the first option too. Any thoughts?
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
