sergio.lists at salvi
Oct 16, 2008, 12:28 PM
Views: 1329
Permalink
|
|
Re: [Catalyst] Race condition in Catalyst::Plugin::Session and Catalyst::Engine::Apache (possibly other engines too)
|
|
Continuing this thread from the catalyst list - original message at
http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/msg04185.html
On Wed, Sep 24, 2008 at 12:05 PM, Matt S Trout <dbix-class[at]trout.me.uk> wrote:
> On Wed, Sep 10, 2008 at 06:59:21PM -0400, Sergio Salvi wrote:
>> There is a race condition in C::P::Session when using
>> C::Engine::Apache (and probably other engines too):
>>
>> I have a simple controller action (let's call it /save) that gets data
>> submitted from an HTML form via POST, process that request, stores
>> some stuff in the session and flash and then redirects with HTTP 303
>> to another action (/display).
>>
>> The /display action then displays the regular "submit successful"
>> message that was set on the previous action by using $c->flash. The
>> problem is that the browser is GETting /display before /save is
>> finished storing the session and flash rows in the database. Then, of
>> course, /display thinks nothing has happened and doesn't display the
>> data from flash.
>>
>> After a bunch of debugging and stack traces :), I figured out the
>> problem is that C::P::Session's finalize() calls $c->NEXT::finalize()
>> before calling $c->finalize_session, so
>> C::Engine::Apache->finalize_body() gets executed *before* the session
>> is flushed in the database, making the browser access /display even
>> though the session may not be stored yet:
>
> This was changed by Bill Moseley in order to fix a bunch of other bugs.
>
> Have a look at the ChangeLog - maybe we could provide an option to reverse
> the order or find another approach?
>
I've checked the ChangeLog and I don't think providing an option to
reverse the order is a safe thing to do. It would break other stuff
and that's why that change was made in the first place.
I couldn't figure out a way to make the Engine finalize() method be
called last, so maybe we should have a specific finalize() method for
Catalyst Engines? Then in Catalyst::handle_request() we do this:
$c->dispatch;
$c->finalize;
$status = $c->finalize_engine;
instead of
$c->dispatch;
$status = $c->finalize;
?
This would also require changing finalize_body, finalize_uploads,
finalize_headers and finalize_cookies so they don't call
$c->engine->finalize_xxx, as the $c->engine->finalize_xxx methods
would be called by $c->finalize_engine.
The finalize_engine method would end up looking almost the same as the
current finalize(), but all method calls would be
$c->engine->finalize_xxx.
How stupid is my idea? Thoughts?
Regards,
Sergio Salvi
_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev
|
Re: [Catalyst] Race condition in Catalyst::Plugin::Session and Catalyst::Engine::Apache (possibly other engines too)
