I would take this patch but we have to be careful to make sure it works with python 2.4. Try prevents
try: except: finally: and the user of "with". On Oct 6, 6:23 pm, ron_m <[email protected]> wrote: > 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, > >
