Re: [Catalyst] Race condition in Catalyst::Plugin::Session and Catalyst::Engine::Apache (possibly other engines too)

2008-09-24 Thread Matt S Trout
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?

-- 
  Matt S Trout   Need help with your Catalyst or DBIx::Class project?
   Technical Directorhttp://www.shadowcat.co.uk/catalyst/
 Shadowcat Systems Ltd.  Want a managed development or deployment platform?
http://chainsawblues.vox.com/http://www.shadowcat.co.uk/servers/

___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


[Catalyst] Race condition in Catalyst::Plugin::Session and Catalyst::Engine::Apache (possibly other engines too)

2008-09-10 Thread Sergio Salvi
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:

# From C::P::Session:

sub finalize {
my $c = shift;
my $ret = $c-NEXT::finalize(@_);

# then finish the rest
$c-finalize_session;
return $ret;
}

I've solved this problem by extending C::P::Session and changing the
behaviour of finalize(), like this:

###

package Catalyst::Plugin::MySession;
use base qw/Catalyst::Plugin::Session/;

use strict;
use warnings;

sub finalize {
my $c = shift;
$c-finalize_session;
my $ret = $c-NEXT::finalize(@_);
return $ret;
}

1;
###

But I realize this may create problems later on if other plugins have
finalize() that modify stuff in the session or flash, because then it
would be too late to modify it as the session/flash was already
stored.

How can I tell Catalyst to call the Engine's finalize() method *last*,
after every other finalize() has been called?

I think that would be the safest way to fix this problem. It is
probably related to C3 MRO, but I'm not sure how to approach this
within Catalyst.

Thank you!
Sergio Salvi

PS: My environment is:

Debian 4.0, stock perl 5.8.8, Apache 2.2.3 with mod_perl 2.0.2
(prefork, no reverse proxy at this moment), MySQL 5.0.32 with InnoDB
and the latest version of major Perl modules:

Catalyst 5.7.014
C::P::Session 0.19
C::P::Session::Store::DBIC 0.06

___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/