[ python-Bugs-924301 ] A leak case with cmd.py & readline &
exception
SourceForge.net
noreply at sourceforge.net
Tue Jun 29 14:59:22 EDT 2004
Bugs item #924301, was opened at 2004-03-27 00:28
Message generated for change (Comment added) made by mwh
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=924301&group_id=5470
Category: Python Library
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Sverker Nilsson (svenil)
Assigned to: Michael Hudson (mwh)
Summary: A leak case with cmd.py & readline & exception
Initial Comment:
A leak to do with readline & cmd, in Python 2.3.
I found out what hold on to my interactive objects
too long ('for ever') in certain circumstances. The
circumstance had to do with an exception being
raised in Cmd.cmdloop and handled (or not handled)
outside of Cmd.cmdloop.
In cmd.py, class Cmd, in cmdloop(), if an
exception is raised and propagated out from the
interior of cmdloop, the function postloop() is
not called. The default function of this, (in 2.3)
when the readline library is present, is to
restore the completer, via:
readline.set_completer(self.old_completer)
If this is not done, the newly (by preloop)
inserted completer will remain. Even if the loop
is called again and run without exception, the new
completer will remain, because then in postloop
the old completer will be set to our new
completer. When we exit, the completer will remain
the one we set. This will hold on to our object,
aka 'leak'. - In cmd.py in 2.2 no attempt was made
to restore the completer, so this was also a kind
of leak, but it was replaced the next time a Cmd
instance was initialized. Now, however, the next
time we will not replace the old completer, but
both of them will remain in memory. The old one
will be stored as self.old_completer. If we
terminate with an exception, bad luck... the
stored completer retains both of the instances. If
we terminate normally, the old one will be
retained. In no case do we restore the space of
the first instance. The only way that would
happen, would be if the second instance first
exited the loop with an exception, and then
entered the loop again an exited normally. But
then, the second instance is retained instead! If
each instance happens to terminate with an
exception, it seems well possible that an ever
increasing chain of leaking instances will be
accumulated.
My fix is to always call the postloop, given the
preloop succeeded. This is done via a try:finally
clause.
def cmdloop(self, intro=None):
...
self.preloop()
try:
...
finally: # Make sure postloop called
self.postloop()
I am attaching my patched version of cmd.py. It
was originally from the tarball of Python 2.3.3
downloaded from Python.org some month or so ago in
which cmd.py had this size & date:
14504 Feb 19 2003 cmd.py
Best regards,
Sverker Nilsson
----------------------------------------------------------------------
>Comment By: Michael Hudson (mwh)
Date: 2004-06-29 19:59
Message:
Logged In: YES
user_id=6656
OK, so I was misreading (or reading an old version, or
something).
I agree with your comments about the bogosities, however
it's not my problem today :-)
Do you think I should check the attached patch in? I do.
----------------------------------------------------------------------
Comment By: Sverker Nilsson (svenil)
Date: 2004-06-23 18:58
Message:
Logged In: YES
user_id=356603
The constructor takes parameters stdin and stdout, and sets
self.stdin and self.stdout from them or from sys.
sys.std[in,out] are then not used directly except implicitly
by raw_input. This seems to have changed somewhere between
Python 2.2 and 2.3. Also, I remember an old version had the
cmdqueue list as a class variable which was not at all
thread safe - now it is an instance variable. I am
hoping/thinking it is thread safe now...
It seems superfluos to have both the use_rawinput
flag and stdin parameter. At least raw_input should not
be used if stdin is some other file than the raw input.
But I don't have a simple suggestion to fix this, for one
thing, it wouldn't be sufficient to compare the stdin
parameter to sys.stdin since that file could have been
changed so it wasn't the raw stdin anymore. Perhaps the
module could store away the original sys.stdin as it was
imported... but that wouldn't quite work since there is no
guarantee sys.stdin hadn't already been changed. I guess if
it is worth the trouble, if someone has an idea, could be a
good thing to fix, anyway...
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2004-06-22 10:07
Message:
Logged In: YES
user_id=6656
Um. Unless I'm *hopelessly* misreading things, cmd.Cmd writes
to sys.stdout unconditionally and either calls raw_input() or
sys.stdin.readline(). So I'm not sure how one would "use a
cmd.Cmd instance in a separate thread, talking eg via a socket
file" without rewriting such huge amounts of the class that thread-
safety becomes your own problem.
Apologies if I'm being dumb... also, please note: I didn't write this
module :-)
----------------------------------------------------------------------
Comment By: Sverker Nilsson (svenil)
Date: 2004-06-22 01:34
Message:
Logged In: YES
user_id=356603
Your comment about threads worries me, I am not sure I
understand it. Would it be unsafe to use a cmd.Cmd instance
in a separate thread, talking eg via a socket file? The
instance is used only by that thread and not by others, but
there may be other threads using other instances. I
understand that it could be unsafe to have two threads share
the same instance, but how about different instances?
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2004-06-21 11:35
Message:
Logged In: YES
user_id=6656
Well, that would seem to be easy enough to fix (see attached).
If you're using cmd.Cmd instances from different threads at the
same time, mind, I think you're screwed anyway. You're certainly
walking on thin ice...
----------------------------------------------------------------------
Comment By: Sverker Nilsson (svenil)
Date: 2004-06-18 20:13
Message:
Logged In: YES
user_id=356603
I think it is OK.
Just noting that it changes the completer (just like my
version) even if use_rawinput is false. I guess one should
remember to pass a null completekey in that case, in case
some other thread was using raw_input. Perhaps a check for
use_rawinput could be added in cmd.py to avoid changing the
completer in that case, for less risk of future mistakes.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2004-06-11 13:29
Message:
Logged In: YES
user_id=6656
yay, that appears to have worked.
let me know what you think.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2004-06-11 13:26
Message:
Logged In: YES
user_id=6656
trying again...
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2004-06-01 11:10
Message:
Logged In: YES
user_id=6656
Bah.
I don't have the laptop with the patch with me, I'll try
uploading again in a couple of days.
----------------------------------------------------------------------
Comment By: Sverker Nilsson (svenil)
Date: 2004-05-29 08:43
Message:
Logged In: YES
user_id=356603
I couldn't find a new attached file.
I acknowledge some problems with my original patch,
but have no other suggestion at the moment.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2004-05-26 17:36
Message:
Logged In: YES
user_id=6656
What do you think of the attached?
This makes the documentation of pre & post loop more accurate
again, which I think is nice.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2004-05-19 10:38
Message:
Logged In: YES
user_id=6656
This is where I go "I wish I'd reviewed that patch more
carefully".
In particular, the documentation of {pre,post}loop is now
out of date.
I wonder setting/getting the completer in these functions
was a good idea. Hmm. This bug report confuses me :-) but
I can certainly see the intent of the patch...
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2004-05-19 02:52
Message:
Logged In: YES
user_id=80475
Michael, this touches some of your code. Do you want to
handle this one?
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=924301&group_id=5470
More information about the Python-bugs-list
mailing list