Re: Different approach to req.get_session().

2005-08-08 Thread Jim Gallacher
Just so everyone is clear, implementation of req.get_session() or its 
equivalent has been deferred to version 3.3.


Graham Dumpleton wrote:

Some will know that I haven't been too keen on the way in which the
proposed new req.get_session() method was being implemented. My concerns
were that it was a magic box and didn't allow access to the underlying
variable holding the session object so as to allow checks such as
determining whether a session already existed before creating it. I also
had my concerns about how the session would be automatically unlocked
when req.internal_redirect() was used, something which may introduce
backward compatibility problems.

Anyway, have been thinking about this stuff on and off and have come
with an idea which might be worth pursuing.

Previously when discussing the issue of the req object not being
available to apache.import_module() unless explicitly provided, and the
implications of this as far as ensuring that settings for logging and
autoreload were consistent between code and Apache configuration files,
I proposed the idea of a request cache.

  http://www.mail-archive.com/python-dev@httpd.apache.org/msg00197.html

This was where the request object was put in a globally accessible
dictionary indexed by thread identifier. Having been done, the request
object could be accessed from anywhere. There were various issues
related to use of req.internal_request() that had to be catered for,
but in short it acted like a thread specific global variable.

Now consider something similar for the instance of a session object
that is created. Ie.,

  def Session(...):

Determine session id if not supplied as argument.

Check session object cache to see if session instance already
stored for (threadId,sessionId).

If cached session object exists, use it and return it.

Otherwise create new session object, store it in the cache under
the key (threadId,sessionId) and return it.

A cleanup handler is already registered for a session object when a
session object is created. This same cleanup handler can remove the
session object from the cache.

In respect of req.internal_redirect(), with the above there doesn't need
to be any special code which unlocks the session. Instead, when any code
executed as a consequence of req.internal_redirect() calls 
Session.Session()

to create the session object, it will be given the already constructed
session object instead of creating a new one. The session lock will never
have been released and the code will simply use it. The code called by
req.internal_redirect() can save the session, or it could even be done
by the code which called req.internal_redirect() in the first place.

Because the session instance is stored in a global cache within the
session code and not in the request object itself, one can probably even
totally do away with the req.get_session() method. Code will simply
continue to use Session.Session() method and users can store it wherever
they like.

Well, thats the basic idea. It obviously needs a bit of work on the caching
with respect to thread locks etc, but that shouldn't be too much work.

Comments? Have I explained it well enough?

Graham



I think this may have merit although I need to mull it over.

Would the session cache be on disk or in memory? If in memory, what are 
the implications in an mpm-prefork environment? If on disk, what are the 
performance implications? Will it introduce new and exciting locking 
issues for accessing the cache?


I'm not looking for answers to the above at this point, just recording 
them for the record.


Jim


Re: Release schedule

2005-08-08 Thread Gregory (Grisha) Trubetskoy


On Mon, 8 Aug 2005, Jim Gallacher wrote:

I had to double escape the r, ie \\\release to make it work. Grisha, did 
you generate the docs on BSD before, and if so is the BSD sed different?


Yes, it was always done on FreeBSD before - it is quite likely that the 
sed is different More likely it's make - the BSD make is _definitely_

different from GNU make.

Grisha


Re: [jira] Updated: (MODPYTHON-69) Potential deadlock in psp cache

2005-08-08 Thread Jim Gallacher
I've committed the fix. For some reason JIRA is picking up the 
subversion commits but not forwarding the message to the mailing list.


This issue can be closed.

Jim

Jim Gallacher (JIRA) wrote:

 [ http://issues.apache.org/jira/browse/MODPYTHON-69?page=all ]

Jim Gallacher updated MODPYTHON-69:
---

Description: 
This issue was discussed on the python-dev mailing list but not followed up on. Fixing that now.


In psp.py 


def dbm_cache_store(srv, dbmfile, filename, mtime, val):

dbm_type = dbm_cache_type(dbmfile)
### potential deadlock here! ###
_apache._global_lock(srv, pspcache)
try:
dbm = dbm_type.open(dbmfile, 'c')
dbm[filename] = %d %s % (mtime, code2str(val))
finally:
try: dbm.close()
except: pass
_apache._global_unlock(srv, pspcache)

pspcache will hash to one of 31 mutexes. Therefore there is a 1 in 31 chance 
for a hash collision if a session is used in the same request, which would result in a 
deadlock. (This has been confirmed by testing.)

Most obvious solution is to use the global lock 0, which will serialize all 
accesses to either pspcache.dbm. Global lock 0 is also used by DbmSession, but 
since the lock is not held for the duration of the request there should not be 
any additional deadlock issues.

The fix is to replace the _apache._global_lock(srv, pspcache) with
_apache._global_lock(srv, None, 0)

The corresponding lock handling in dbm_cache_get() will also need the same fix.

I will commit the this fix shortly.



  was:
This issue was discussed on the python-dev mailing list but not followed up on. 
Fixing that now.

In psp.py 


def dbm_cache_store(srv, dbmfile, filename, mtime, val):

dbm_type = dbm_cache_type(dbmfile)
### potential deadlock here! ###
_apache._global_lock(srv, pspcache)
try:
dbm = dbm_type.open(dbmfile, 'c')
dbm[filename] = %d %s % (mtime, code2str(val))
finally:
try: dbm.close()
except: pass
_apache._global_unlock(srv, pspcache)

pspcache will hash to one of 31 mutexes. Therefore there is a 1 in 31 chance 
for a hash collision if a session is used in the same request, which would result in a 
deadlock. (This has been confirmed by testing.)

Most obvious solution is to use the global lock 0, which will serialize all 
accesses to either pspcache.dbm. Global lock 0 is also used by DbmSession, but 
since the lock is not held for the duration of the request there should not be 
any additional deadlock issues.

The fix is to replace the _apache._global_lock(srv, pspcache) with
_apache._global_lock(srv, None, 0)

I will commit the this fix shortly.






Potential deadlock in psp cache
---

Key: MODPYTHON-69
URL: http://issues.apache.org/jira/browse/MODPYTHON-69
Project: mod_python
   Type: Bug
 Components: publisher
   Versions: 3.2.0
Environment: All
   Reporter: Jim Gallacher




This issue was discussed on the python-dev mailing list but not followed up on. 
Fixing that now.
In psp.py 
def dbm_cache_store(srv, dbmfile, filename, mtime, val):

   dbm_type = dbm_cache_type(dbmfile)
   ### potential deadlock here! ###
   _apache._global_lock(srv, pspcache)
   try:
   dbm = dbm_type.open(dbmfile, 'c')
   dbm[filename] = %d %s % (mtime, code2str(val))
   finally:
   try: dbm.close()
   except: pass
   _apache._global_unlock(srv, pspcache)
pspcache will hash to one of 31 mutexes. Therefore there is a 1 in 31 chance 
for a hash collision if a session is used in the same request, which would result in a 
deadlock. (This has been confirmed by testing.)
Most obvious solution is to use the global lock 0, which will serialize all 
accesses to either pspcache.dbm. Global lock 0 is also used by DbmSession, but 
since the lock is not held for the duration of the request there should not be 
any additional deadlock issues.
The fix is to replace the _apache._global_lock(srv, pspcache) with
_apache._global_lock(srv, None, 0)
The corresponding lock handling in dbm_cache_get() will also need the same fix.
I will commit the this fix shortly.