[Pagekit-devel] delete session and page_session before pkit_cleanup_code

[Boris Z. <bo...@m2...>]

Hi,

Anton Berezin found a bug in the PageKit handler. Here is his bug analysis:

> I might just have discovered a rather subtle bug in 1.07.  I am unsure
> about the proper fix, because the bug is related to the sequence of
> various cleanup calls PageKit performs, and it might be unwise to change
> the said sequence.
>
> At any rate, here is the description:
>
> The PageKit guide mentions that pkit_cleanup_code() is useful for doing
> DBI disconnect.  It also mentions that it might be overall a better idea
> to use Apache::DBI instead.  If Apache::DBI is not used, however, and
> the session object uses the same DBI handler, we have a problem:
>
> The pkit_cleanup_code() is called *before* delete $pk->{session} (in
> Apache::PageKit::handler).  If the session was modified, its destruction
> will trigger the DB update using the DB handler just disconnect()ed.
>
> In the best case, this is an error, and the session will not be updated.
>
> In the worst case, which I actually observed in practice, DBI will fail
> to perform the necessary checks, and will call appropriate DBD
> functions.  In the particular case of Postgresql, the pointer to already
> free()d pg_conn structure is passed to PQexec() (something which is very
> difficult, if at all possible, to detect), with the resulting tight
> endlsess loop.
>
> Possible solutions:
>
> 1. Leave everything as is, but modify the guide to tell that Apache::DBI
>    is *required* for normal operation.
>
> 2. Delete the session before calling pkit_cleanup_code(), and modify the
>    guide to tell that the session is invalid inside pkit_cleanup_code().
>
> 3. Add another method, named, for example, pkit_finalize_code(), which
>    is called after the session deletion.
>
> There may be other way to fix this - this is of course up to you to
> decide.  It is just that I really think that it would be very nice to
> fix this problem before 1.08 is released.
>
> I've spent a day yesterday finding this bug.  The debugging included
> repeated recompilation of DBI, DBD::Pg, and Postgresql itself with some
> extra debug prints.  :-(
>
> Cheers,
> &Anton.

i found his option 2. best. This may affect your code, if you use pkit_cleanup_code
and use $model->session inside this hook. Move your code or part of it to 
pkit_post_common_code.

untested patch applied.

Thanks goto Anton for digging this out!

-- 
cu boris

In Bayern haben Forscher endeckt, das im Bier weibliche Gene vorhanden sind.
Beweisführung: An 1000 männlichen Probanden wurden je 30 Flaschen
Bier ausgegeben und von diesen getrunken, danach zeigten sich
eindeutig weibliche Eigenschaften.  Keiner konnte mehr Autofahren und
alle redeten nur noch dummes Zeug.