[pmwiki-users] Session variable issues

Joachim Durchholz jo at durchholz.org
Sat Jul 9 16:18:19 CDT 2005


Patrick R. Michaud wrote:

> On Sat, Jul 09, 2005 at 11:25:48AM +0200, Joachim Durchholz wrote:
> 
>>Patrick R. Michaud wrote:
>>
>>>Yes; for some reason PHP complains with a warning if session_start
>>>is called twice.  (Personally, I wish it would just be a no-op if
>>>the session has already started.  And I also wish there was a way
>>>to know if a session has already started, but PHP doesn't seem to
>>>tell us that either.)
>>
>>Hmm... the session handling code in PmWiki seems rather fragile to me. 
> 
> 
> It's PHP's session handling code that is fragile, and as a result
> PmWiki has to work around it.  I'll explain...
> 
> 
>>I'd suggest keeping the PHP session state in a global variable. I.e. 
>>something like
>>
>>$SessionActive = 0;
>>
>>function OpenSession() {
>>  if (!$SessionActive) {
>>    session_start();
>>    $SessionActive = 1;
>>  }
>>}
>>
>>function CloseSession() {
>>  if ($SessionActive) {
>>    session_write_close();
>>    $SessionActive = 0;
>>  }
>>}
> 
> This isn't really robust, for several reasons.  First, it can only
> work as long as every PmWiki script (including cookbook recipes and
> local customizations) uses the OpenSession/CloseSession functions 
> instead of the built-in PHP session functions.  

True, but that's not a real problem - opening and closing sessions is a 
policy-ridden issue anyway. E.g. you may want to change the session id 
on every call to avoid session hijacking. Or allow session splitting 
(i.e. make the "back" button on the browsers work, and allow a session 
to be split in mulitple browser windows).
All this besides getting sessions right for every PHP and server version 
in the world. Plus getting it right with cookies and with GET parameters 
(PHP does *most* of the work for you, but some things remain to be done).

This all requires some moderate lifting, and people wouldn't want to 
duplicate that code, so some PmWiki-defined functions for accessing 
session data is fully in order IMHO.

> But more importantly, as written above it's too easy for a function
> to have something else unexpectedly and indirectly close the session.  
> For example, consider the code:
> 
>     OpenSession();
>     $foo = $_SESSION['foo'];
>     $page = RetrieveAuthPage($pagename, 'read', false);
>     $html = MarkupToHTML($pagename, $page['text']);
>     # ... other stuff goes here...
>     $_SESSION['foo'] = $foo;
>     CloseSession();
> 
> Looking at the above one would normally expect the new value of 
> foo to be saved in the session before it closes, but it isn't.  The
> problem is that RetrieveAuthPage() also uses session variables,
> so it will have called CloseSession() as well, thus the session is
> no longer open after retrieving the page.  (And perhaps some of
> the markups in the MarkupToHTML are making use of OpenSession()/
> CloseSession() too!)

Ah, right. My bad.

Easily fixed though:

$SessionActive = 0;

function OpenSession() {
   if ($SessionActive <= 0) {
     session_start();
   }
   $SessionActive = $SessionActive + 1;
}

function CloseSession() {
   if ($SessionActive > 0) {
     $SessionActive = $SessionActive - 1;
     if ($SessionActive == 0) {
       session_write_close();
     }
   }
}

Now nested OpenSession() - CloseSession() calls will do the Right Thing.

> So, to be robust a script author must wrap every individual
> access to $_SESSION in an OpenSession/CloseSession pair, which seems
> to add a lot of serialization overhead to the system.

No overhead with $SessionActive as a level count.

 > Or, we try to
> have OpenSession and CloseSession keep track of the number of 
> active opens, and actually execute session_write_close only 
> on the closing of the last active open/close pair.

Exactly what the above does. (I should read the rest of the mail before 
answering it...)

> But PmWiki currently takes a different approach, which is to close
> sessions only when it's known to be safe to do so, or when closing
> the session is necessary to avoid deadlocks.

That's a fragile approach.

The question whether a deadlock can occur is a global one: entirely 
different parts of the PmWiki code can incur deadlock.
Besides, session data cannot cause deadlock between different sessions; 
it's only relevant if two pages are rendered with the same session data, 
i.e. when doing framesets. PmWiki internally serialises any rendering 
anyway, so avoiding deadlocks is a non-issue.

 > It's safe for a
> function to close a session if it knows that no session was open
> when the function started.  This is what PmWikiAuth() does.
> Unfortunately, PHP doesn't give us a way to know if a session is
> currently open -- the best PHP can do is tell us if a session has
> *ever* been opened (even if that session has since been closed).  

That's why I think that there should be functions that track whether a 
session was opened. (If PHP doesn't tell us, we have to do the 
bookkeeping ourselves.)

> So, if PmWikiAuth() recognizes that it opened the first session 
> it goes ahead and closes it as a likely optimization, but for 
> the second and subsequent sessions it leaves it open in case some 
> previous code is relying on the session being open.
> 
> We also sometimes have to issue session_write_close to avoid
> deadlock issues, which is what happens in the Lock() function.
> 
> But ultimately, calling session_write_close() a bunch of times during
> a script doesn't give a whole lot of benefit.  At some point it 
> becomes wiser to say "leave the session open for the remainder 
> of the script execution" than to try to actively manage the 
> session state, which is what
> 
>    @session_start();
> 
> effectively does.

Just leaving the session open for PHP to close will not work if there's 
a redirect involved.

Of course, that's easy to solve in Redirect.

Regards,
Jo



More information about the pmwiki-users mailing list