From 809a4eb2493a9757eaab3e1e4d47ebe26457754a Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 29 Feb 2020 10:54:16 +0000 Subject: [PATCH] testcrypt.py: avoid restarting subprocess for frees. I just ran into a bug in which the testcrypt child process was cleanly terminated, but at least one Python object was left lying around containing the identifier of a testcrypt object that had never been freed. On program exit, the Python reference count on that object went to zero, the __del__ method was invoked, and childprocess.funcall started a _new_ instance of testcrypt just so it could tell it to free the object identifier - which, of course, the new testcrypt had never heard of! We can already tell the difference between a ChildProcess object which has no subprocess because it hasn't yet been started, and one which has no subprocess because it's terminated: the latter has exitstatus set to something other than None. So now we enforce by assertion that we don't ever restart the child process, and the __del__ method avoids doing anything if the child has already finished. --- test/testcrypt.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/testcrypt.py b/test/testcrypt.py index 9fb35d2d..7412fcc7 100644 --- a/test/testcrypt.py +++ b/test/testcrypt.py @@ -69,8 +69,11 @@ class ChildProcess(object): if self.debug is not None: self.debug.write("recv: {}\n".format(line)) return line + def already_terminated(self): + return self.sp is None and self.exitstatus is not None def funcall(self, cmd, args): if self.sp is None: + assert self.exitstatus is None self.start() self.write_line(unicode_to_bytes(cmd) + b" " + b" ".join( unicode_to_bytes(arg) for arg in args)) @@ -117,7 +120,7 @@ class Value(object): def __repr__(self): return "Value({!r}, {!r})".format(self._typename, self._ident) def __del__(self): - if self._ident is not None: + if self._ident is not None and not childprocess.already_terminated(): try: childprocess.funcall("free", [self._ident]) except ChildProcessFailure: