Glad to help. While looking in the code I noticed something else and thought I should ask. Jason had also mentioned a problem he had with running out of file table slots - the server could not open any more files. This is likely related to the session problem he experienced.
The Python reference manual talks about closing a file in a finally block or in Python >= 2.5 using the file object returned from open in a with context to guarantee close gets called. I would have thought a file would be closed in the library if it got unrefed when response went out of scope at the end of the request cycle since session_file is an attribute of response. This of course would depend on garbage collect calling the file object destructor if this even happens. I haven't read the Python library to know. Since Jason actually observed running out of file table slots I would guess there is no automatic destructor based close but instead a dependence on the process exit where a close all files occurs when a C program (the Python interpreter) exits from main(). Applying this to web2py I can see a couple of problems. Normally after the first request for the session, the file is opened in session._connect called from inside a try block in main.py. If the cPickle.load fails the file is unlocked and abandoned but not closed. This is possibly the failure situation Jason was seeing. Later in the except HTTP, http_response branch in main.py session._try_store_on_disk() is called which will close the file provided portalocker.unlock doesn't throw an exception. Proposed enhancement The with context method won't work because open is called from a try block and close is called from an except block related to the try in main.py so the file would be closed before it needs to be written to. That leaves finally as a possible solution but that won't work for a normal request, only when cPickle.load throws an exception a close is needed for the exception case. To cover off all the other possible ways out between the _connect and the _try_store_on_disk maybe a finally clause could be put on the end of the try except block that calls both of the above functions and inside a test to see if the file has been opened and a try block to close the file with an except with just pass to eat anything that happens there. I can put together a patch to trunk if you like? Ron On Oct 6, 6:47 am, mdipierro <[email protected]> wrote: > In trunk! thanks ron! > > On Oct 6, 3:53 am, Jason Brower <[email protected]> wrote: > > > Well, I tried the change that you had requested. And, IT WORKED, I made > > the change and the session doesn't get reset and I don't get the errors > > at all anymore! Just to make sure I put the file back and tried it > > again. And sure enough the issue came back again. Very cool to see > > that happen. And your right, I didn't realize the issue at hand, but now > > I understand. Thank you! Are you gong to file a patch for this one? > > Best regards, > > Jason Brower > > > On 10/05/2010 07:06 PM, ron_m wrote: > > > > I think you partially missed my point. All it would take to have a > > > shorter session pickle file on a request is a shorter string stored in > > > one of the session variables. Normally a pickle dump is done with a > > > file open in mode 'wb' but in the server when a session file already > > > exists for the session it is opened 'rb+' to read the previously > > > stored data at the start of the request cycle then rewound to offset 0 > > > and written with the new session data at the end of the web request > > > cycle. If the session data is shorter for any reason the next request > > > will get a pickle file with this data and some extra data from 2 or > > > more requests ago because the end of file was not moved to match the > > > actual data length by a truncate call. The extra data on the end may > > > or may not be a problem depending on how cPickle reads back the file. > > > If cPickle.load were to detect a problem then the session is abandoned > > > and a new one is used, It should be easy enough for you to apply the > > > one line patch to your system, restart the server and run your tests > > > to see if this fixes the problem. > > > > On Oct 5, 3:18 am, Jason Brower<[email protected]> wrote: > > > >> Interesting Observation about the trancating. This could save some > > >> space although I think the case is rare. :/ > > >> I will continue examine the session data to see if there is anything > > >> else that could cause this... > > >> Best Regards, > > >> Jason > > > >> On 10/05/2010 11:40 AM, ron_m wrote: > > > >>> If you store something in session as in session.name = some_variable > > >>> then that will be pickled at the end of the current request along with > > >>> anything else in session. The result of the pickle operation is the > > >>> session file for your session containing all your session variables. > > > >>> On the next request the session file is unpickled to restore the > > >>> session variables for your session. This is the only way you can > > >>> retain state between requests. > > > >>> If a session variable contains a reference to a class object instance > > >>> it will likely pickle with success. However, when you hit the next > > >>> request possibly in a different controller and are missing a class > > >>> definition because of a missing import in the other controller the > > >>> unpickle will fail. > > > >>> You have to look at every session.something that has an assignment to > > >>> it and then look inside the right hand side of that assignment for > > >>> what is inside. It could even be a bug where a typo in your logic is > > >>> assigning a class instance into a place in a list or dictionary which > > >>> is then placed in the session. A Google search for "insecure string > > >>> pickle" reveals many causes such as pickle on Windows and unpickle on > > >>> UNIX because line endings in Windows are \r\n but on UNIX are \n > > >>> unless binary file format is used. > > > >>> However, have a look at this first, sorry for rambling a bit: > > > >>> I took a look at gluon/globals.py where request, response and session > > >>> are classes. I am suspicious of something in the session saving. > > > >>> If the session file already exists at the start of the request cycle > > >>> the session file is opened in mode 'rb+', the session file is locked > > >>> and then it is unpickled with cPickle.load and then the file is seeked > > >>> to offset 0 to rewind the position in preparation for the session > > >>> write. > > > >>> Later at the end of the request cycle the function _try_store_on_disk > > >>> for an already existing session file has the current version of > > >>> session pickled into it with cPickle.dump, then the session file is > > >>> unlocked and closed. > > > >>> What I am unsure of is if the session data on the current request to > > >>> use the session is smaller than the session data from the last request > > >>> then there will be some left over cruft on the end of the file from > > >>> the previous session. Maybe the file should be truncated at the > > >>> current position after the cPickle.dump and before the close. I don't > > >>> know the internal structure of a pickle file well enough to know if > > >>> end of file is important and whether or not extra data on the end of > > >>> the file will fool the next unpickle or load call resulting in the > > >>> error you are seeing. > > > >>> To test this you could look for line 378 (Rev 1.86.2) in gluon/ > > >>> globals.py which is the cPickle.dump line > > >>> if response.session_file: > > >>> cPickle.dump(dict(self), response.session_file) > > > >>> and add this line of code after the cPickle.dump line > > >>> response.session_file.truncate() > > > >>> at the same indent level as the cPickle.dump line. This will drop the > > >>> file size down to the current size of data from the current request > > >>> session write if the session data has shrunk and do nothing otherwise. > > > >>> Massimo, what do you think of this? > > > >>> Ron > > > >>> On Oct 4, 11:30 pm, Jason Brower<[email protected]> wrote: > > > >>>> Looked at all the data I would ever send or use in the system and I > > >>>> don't see any pickles. I only use them in the messages I am sending > > >>>> and > > >>>> then unpickle them as soon as they get to me. I can't imagine were else > > >>>> it could be as I don't play with any funny objects or persistent items. > > >>>> Additionally, the updates only happen every 2 seconds. I will get > > >>>> good, > >
