|
Home > Archive > Apache Mod-Python > June 2005 > Potential deadlock in psp.py
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
| Author |
Potential deadlock in psp.py
|
|
| Jim Gallacher 2005-06-23, 5:50 pm |
| I think I just spotted a potential deadlock in psp.py.
def dbm_cache_store(srv, dbmfile, filename, mtime, val):
dbm_type = dbm_cache_type(dbmfile)
_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.
I'm sure there are other possible deadlock scenarios where some
combination of sessions and pspcache could end up trying to hold the
same mutex.
Most obvious solution is to use the global lock 0, which will serialize
all accesses to either pspcache.dbm (and the dbmsession dbm in the case
where DbmSession is being used). This will result in an obvious
performance hit when psp are used in conjunction with DbmSession, but
that's better than a deadlock.
_apache._global_lock(srv, None, 0)
The alternative would be to reserve a mutex for pspcache.
If you agree that this is a possible deadlock let me know and I'll make
the simple fix. We could also take another look at the bsddb transaction
handling discussed last week in my Session benchmark post, and avoid
using the mutex altogether.
Regards,
Jim
| |
| Gregory (Grisha) Trubetskoy 2005-06-23, 5:50 pm |
|
Is this reproducable? With a 1 in 31 chance it should be pretty easy to
reproduce... Just want to make sure that there isn't some undrelying
reason why this was done this way.
Grisha
On Thu, 23 Jun 2005, Jim Gallacher wrote:
> I think I just spotted a potential deadlock in psp.py.
>
> def dbm_cache_store(srv, dbmfile, filename, mtime, val):
>
> dbm_type = dbm_cache_type(dbmfile)
> _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.
>
> I'm sure there are other possible deadlock scenarios where some combination
> of sessions and pspcache could end up trying to hold the same mutex.
>
> Most obvious solution is to use the global lock 0, which will serialize all
> accesses to either pspcache.dbm (and the dbmsession dbm in the case where
> DbmSession is being used). This will result in an obvious performance hit
> when psp are used in conjunction with DbmSession, but that's better than a
> deadlock.
>
> _apache._global_lock(srv, None, 0)
>
> The alternative would be to reserve a mutex for pspcache.
>
> If you agree that this is a possible deadlock let me know and I'll make the
> simple fix. We could also take another look at the bsddb transaction handling
> discussed last week in my Session benchmark post, and avoid using the mutex
> altogether.
>
> Regards,
> Jim
>
| |
| Jim Gallacher 2005-06-23, 5:50 pm |
| Gregory (Grisha) Trubetskoy wrote:
>
> Is this reproducable? With a 1 in 31 chance it should be pretty easy to
> reproduce... Just want to make sure that there isn't some undrelying
> reason why this was done this way.
The supposition that there is a deadlock is untested and simply based on
inspecting the code. dharana was asking about using psp outside of
apache, and when I took a look at psp.py I noticed the potential
deadlock. I'll give it a whirl and see if I can lock up my server. 
Jim
> Grisha
>
> On Thu, 23 Jun 2005, Jim Gallacher wrote:
>
>
| |
| Nicolas Lehuen 2005-06-23, 5:50 pm |
| Hi Jim,
Until now, we suspected that the way global locks are handled could be
deadlock prone. You have just proved it.
I know that global locks are expensive on some systems, especially if
we want to use them in a multiprocess (forked) environment. That's why
we are forced to have such a small pool of global locks.
On the other hand, in a multithreaded environment, locks which are
valid for the whole process are not so expensive ; indeed, we can
create a whole bunch without bringing down the system (think about
Java where all object potentially have a monitor which is equivalent
to a lock).
I think this is a strong incentive to abandon the forked model and go
for the multi-threaded model (or the mono-threaded, asynchronous one).
For concurrency control, the forked model does not scale, apparently.
Regards,
Nicolas
2005/6/23, Jim Gallacher <jg.lists@sympatico.ca>:
> I think I just spotted a potential deadlock in psp.py.
>=20
> def dbm_cache_store(srv, dbmfile, filename, mtime, val):
>=20
> dbm_type =3D dbm_cache_type(dbmfile)
> _apache._global_lock(srv, "pspcache")
> try:
> dbm =3D dbm_type.open(dbmfile, 'c')
> dbm[filename] =3D "%d %s" % (mtime, code2str(val))
> finally:
> try: dbm.close()
> except: pass
> _apache._global_unlock(srv, "pspcache")
>=20
> "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.
>=20
> I'm sure there are other possible deadlock scenarios where some
> combination of sessions and pspcache could end up trying to hold the
> same mutex.
>=20
> Most obvious solution is to use the global lock 0, which will serialize
> all accesses to either pspcache.dbm (and the dbmsession dbm in the case
> where DbmSession is being used). This will result in an obvious
> performance hit when psp are used in conjunction with DbmSession, but
> that's better than a deadlock.
>=20
> _apache._global_lock(srv, None, 0)
>=20
> The alternative would be to reserve a mutex for pspcache.
>=20
> If you agree that this is a possible deadlock let me know and I'll make
> the simple fix. We could also take another look at the bsddb transaction
> handling discussed last week in my Session benchmark post, and avoid
> using the mutex altogether.
>=20
> Regards,
> Jim
>
| |
| Jim Gallacher 2005-06-23, 5:50 pm |
| Jim Gallacher wrote:
> Gregory (Grisha) Trubetskoy wrote:
>
>
>
> The supposition that there is a deadlock is untested and simply based on
> inspecting the code. dharana was asking about using psp outside of
> apache, and when I took a look at psp.py I noticed the potential
> deadlock. I'll give it a whirl and see if I can lock up my server.
Confirmed and reproducable.
Using mpm-worker with 2 servers started, deadlock occurs within 10 to 50
requests based on 5 test runs
ab -n 100 http://localhost/mp/mptest.py.
Bad handler - deadlock
----------------------
def handler(def):
req.content_type = 'text/plain'
sess = Session.FileSession(req)
template = psp.PSP(req, filename='/var/www/mp/template.psp')
template.run({'what':'world'})
return apache.OK
Good handler - no deadlock when session is unlocked
---------------------------------------------------
def handler(def):
req.content_type = 'text/plain'
sess = Session.FileSession(req)
sess.unlock()
template = psp.PSP(req, filename='/var/www/mp/template.psp')
template.run({'what':'world'})
return apache.OK
template.psp
------------
psp test
<%= what %>
End test
Jim
| |
| Jim Gallacher 2005-06-23, 5:50 pm |
| Nicolas Lehuen wrote:
> Hi Jim,
>
> Until now, we suspected that the way global locks are handled could be
> deadlock prone. You have just proved it.
>
> I know that global locks are expensive on some systems, especially if
> we want to use them in a multiprocess (forked) environment. That's why
> we are forced to have such a small pool of global locks.
>
> On the other hand, in a multithreaded environment, locks which are
> valid for the whole process are not so expensive ; indeed, we can
> create a whole bunch without bringing down the system (think about
> Java where all object potentially have a monitor which is equivalent
> to a lock).
>
> I think this is a strong incentive to abandon the forked model and go
> for the multi-threaded model (or the mono-threaded, asynchronous one).
> For concurrency control, the forked model does not scale, apparently.
It would make life easier I suppose, but we are stuck with the fact that
apache provides several different mpm models that need to be supported.
Jim
| |
| Gregory (Grisha) Trubetskoy 2005-06-24, 2:45 am |
|
Yeah, we've got to be inline with the HTTP Project - prefork is the
default on unix systems, so we have to abide by it...
So I guess the solution is that we need to reserve two locks instead of
just one?
Grisha
On Thu, 23 Jun 2005, Jim Gallacher wrote:
> Nicolas Lehuen wrote:
>
> It would make life easier I suppose, but we are stuck with the fact that
> apache provides several different mpm models that need to be supported.
>
> Jim
>
| |
| Nicolas Lehuen 2005-06-24, 2:45 am |
| Well, why not keep the 32 session locks for sessions, and add
something to the locking API so that we can create dedicated locks for
dedicated usages (the session database lock, the PSP cache lock...) ?
The next time someone will need a dedicated lock, he will be tempted
to rely on the current implementation, thus creating new potential for
deadlocks...
Regards,
Nicolas
2005/6/24, Gregory (Grisha) Trubetskoy <grisha@apache.org>:
>=20
> Yeah, we've got to be inline with the HTTP Project - prefork is the
> default on unix systems, so we have to abide by it...
>=20
> So I guess the solution is that we need to reserve two locks instead of
> just one?
>=20
> Grisha
>=20
> On Thu, 23 Jun 2005, Jim Gallacher wrote:
>=20
t[vbcol=seagreen]
>
| |
| Jim Gallacher 2005-06-24, 7:47 am |
| Gregory (Grisha) Trubetskoy wrote:
>
> Yeah, we've got to be inline with the HTTP Project - prefork is the
> default on unix systems, so we have to abide by it...
>
> So I guess the solution is that we need to reserve two locks instead of
> just one?
It seems a shame to waste locks if they are not going to be used. I
don't think there is a deadlock problem if lock index 0 is used for both
DbmSession file locking and psp cache file locking. In both cases the
lock is held for a short time. I'm not sure what impact this would have
on performance, but I guess it could become a bottleneck.
Regards,
Jim
> Grisha
>
> On Thu, 23 Jun 2005, Jim Gallacher wrote:
>
>
| |
| Gregory (Grisha) Trubetskoy 2005-06-24, 5:46 pm |
|
On Fri, 24 Jun 2005, Jim Gallacher wrote:
> Gregory (Grisha) Trubetskoy wrote:
>
> It seems a shame to waste locks if they are not going to be used. I don't
> think there is a deadlock problem if lock index 0 is used for both DbmSession
> file locking and psp cache file locking. In both cases the lock is held for a
> short time. I'm not sure what impact this would have on performance, but I
> guess it could become a bottleneck.
I'm +1 on this (assuming it works). I don't think this is going to be a
performance problem.
Grisha
|
|
|
|
|