[mod_python] Session.py: filesession_cleanup (I)

David Janes wrote:
> I'm getting random hangs on a mod_python based system under moderately 
> load. I haven't tracked down the problem yet, but based on some error 
> logging I've been doing code inspection and have a few comments/questions.
> 
> (1)
> In Session.py, function filesession_cleanup, lines 624-638 there's a 
> try/except block that returns on an exception. However, to get to this 
> block, 'lockfp' has to be open -- and there is no close in the exception 
> block.
> 
> Should not this code block be moved inside the next try/finally block 
> which ensures the file object is closed?

I think it's ok as written. The purpose of the lockfile is to make sure 
that another process is not already running the cleanup.

lockfp = os.open(lockfile, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0660)

The lockfile is being opened with the os.O_EXCL flag, so if another 
process has the file open *this* process will fail to open the file and 
raise an exception.

I can see why there might be some confusion though as the try/except 
block is doing double duty - making sure another cleanup is not already 
running *and* removing a potentially stale lockfile. Short circuiting 
the cleanup in the case of a stale lock file here (with the return) is 
not really that big of a deal. It's not imperative that we run the 
cleanup *right now*, just so long as the cleanup happens in the near 
future where "near" is a pretty fuzzy number.

> (2)
> In my logs I am getting the error message
> 
> python_cleanup: Error calling cleanup object <function 
> filesession_cleanup at 0xb7b22924>
> exceptions.TypeError: not enough arguments for format string
> 
> The likely culprit for this error is line 705 (-707):
> 
>             req.log_error("FileSession cleanup incomplete: next cleanup 
> will start at index %d (%02x)"
>                         % (next_i,),
>                         apache.APLOG_NOTICE)
> 
> Does this make sense?

As I previously noted that one is a bug and should read:

req.log_error("FileSession cleanup incomplete: next cleanup will start 
at index %d (%02x)"
                         % (next_i, next_i),
                         apache.APLOG_NOTICE)



I'll fix it in svn.

Just out of curiosity how many session objects do you figure you have?

Jim