Tell me when the patch is in, I am eager to try it.
BR,
Jason
On 10/07/2010 02:41 AM, mdipierro wrote:
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<ron.mco...@gmail.com> 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<mdipie...@cs.depaul.edu> wrote:
In trunk! thanks ron!
On Oct 6, 3:53 am, Jason Brower<encomp...@gmail.com> 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<encomp...@gmail.com> 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<encomp...@gmail.com> 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,
