* Fix a refleak when a preexec_fn was supplied (preexec_fn_args_tuple was not
being defref'ed).
* Fixes another potential refleak of a reference to the gc
module in the unlikely odd case where gc module isenabled or disable calls
fail.
* Adds a unittest for the above case to verify behavior and lack of leaks.
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 74d9ce4..9f8512d 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -9,6 +9,10 @@
import time
import re
import sysconfig
+try:
+ import gc
+except ImportError:
+ gc = None
mswindows = (sys.platform == "win32")
@@ -650,6 +654,44 @@
self.fail("Exception raised by preexec_fn did not make it "
"to the parent process.")
+ @unittest.skipUnless(gc, "Requires a gc module.")
+ def test_preexec_gc_module_failure(self):
+ # This tests the code that disables garbage collection if the child
+ # process will execute any Python.
+ def raise_runtime_error():
+ raise RuntimeError("this shouldn't escape")
+ enabled = gc.isenabled()
+ orig_gc_disable = gc.disable
+ orig_gc_isenabled = gc.isenabled
+ try:
+ gc.disable()
+ self.assertFalse(gc.isenabled())
+ subprocess.call([sys.executable, '-c', ''],
+ preexec_fn=lambda: None)
+ self.assertFalse(gc.isenabled(),
+ "Popen enabled gc when it shouldn't.")
+
+ gc.enable()
+ self.assertTrue(gc.isenabled())
+ subprocess.call([sys.executable, '-c', ''],
+ preexec_fn=lambda: None)
+ self.assertTrue(gc.isenabled(), "Popen left gc disabled.")
+
+ gc.disable = raise_runtime_error
+ self.assertRaises(RuntimeError, subprocess.Popen,
+ [sys.executable, '-c', ''],
+ preexec_fn=lambda: None)
+
+ del gc.isenabled # force an AttributeError
+ self.assertRaises(AttributeError, subprocess.Popen,
+ [sys.executable, '-c', ''],
+ preexec_fn=lambda: None)
+ finally:
+ gc.disable = orig_gc_disable
+ gc.isenabled = orig_gc_isenabled
+ if not enabled:
+ gc.disable()
+
def test_args_string(self):
# args is a string
fd, fname = mkstemp()
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index f9b38b6..a6008ef 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -204,15 +204,21 @@
if (gc_module == NULL)
return NULL;
result = PyObject_CallMethod(gc_module, "isenabled", NULL);
- if (result == NULL)
+ if (result == NULL) {
+ Py_DECREF(gc_module);
return NULL;
+ }
need_to_reenable_gc = PyObject_IsTrue(result);
Py_DECREF(result);
- if (need_to_reenable_gc == -1)
+ if (need_to_reenable_gc == -1) {
+ Py_DECREF(gc_module);
return NULL;
+ }
result = PyObject_CallMethod(gc_module, "disable", NULL);
- if (result == NULL)
+ if (result == NULL) {
+ Py_DECREF(gc_module);
return NULL;
+ }
Py_DECREF(result);
}
@@ -307,6 +313,7 @@
Py_XDECREF(gc_module);
return NULL;
}
+ Py_XDECREF(preexec_fn_args_tuple);
Py_XDECREF(gc_module);
if (pid == -1)
@@ -322,6 +329,7 @@
_Py_FreeCharPArray(exec_array);
Py_XDECREF(converted_args);
Py_XDECREF(fast_args);
+ Py_XDECREF(preexec_fn_args_tuple);
/* Reenable gc if it was disabled. */
if (need_to_reenable_gc)