From babbe0870633f4210da811b121c95f6ea65ded8a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 20 Jun 2018 16:52:08 +0200 Subject: [PATCH 1/2] bpo-18174: regrtest -R 3:3 checks for handle leak * Add _winapi.GetProcessHandleCount() * regrtest now also checks for leak of Windows handles * Add an unit test in test_regrtest Co-Authored-By: Richard Oudkerk --- Lib/test/libregrtest/refleak.py | 20 ++++++++--- Lib/test/test_regrtest.py | 22 ++++++++++++ .../2018-06-20-16-55-12.bpo-18174.343gmT.rst | 2 ++ Modules/_winapi.c | 28 +++++++++++++++ Modules/clinic/_winapi.c.h | 34 ++++++++++++++++++- 5 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2018-06-20-16-55-12.bpo-18174.343gmT.rst diff --git a/Lib/test/libregrtest/refleak.py b/Lib/test/libregrtest/refleak.py index 6724488fcfb088..901334584da6c1 100644 --- a/Lib/test/libregrtest/refleak.py +++ b/Lib/test/libregrtest/refleak.py @@ -13,6 +13,12 @@ def _get_dump(cls): return (cls._abc_registry, cls._abc_cache, cls._abc_negative_cache, cls._abc_negative_cache_version) +if sys.platform == 'win32': + from _winapi import GetProcessHandleCount as handle_count +else: + def handle_count(): + return 0 + def dash_R(the_module, test, indirect_test, huntrleaks): """Run a test multiple times, looking for reference leaks. @@ -59,24 +65,27 @@ def get_pooled_int(value): rc_deltas = [0] * repcount alloc_deltas = [0] * repcount fd_deltas = [0] * repcount + handle_deltas = [0] * repcount print("beginning", repcount, "repetitions", file=sys.stderr) print(("1234567890"*(repcount//10 + 1))[:repcount], file=sys.stderr, flush=True) # initialize variables to make pyflakes quiet - rc_before = alloc_before = fd_before = 0 + rc_before = alloc_before = fd_before = handle_before = 0 for i in range(repcount): indirect_test() - alloc_after, rc_after, fd_after = dash_R_cleanup(fs, ps, pic, zdc, - abcs) + result = dash_R_cleanup(fs, ps, pic, zdc, abcs) + alloc_after, rc_after, fd_after, handle_after = result print('.', end='', file=sys.stderr, flush=True) if i >= nwarmup: rc_deltas[i] = get_pooled_int(rc_after - rc_before) alloc_deltas[i] = get_pooled_int(alloc_after - alloc_before) fd_deltas[i] = get_pooled_int(fd_after - fd_before) + handle_deltas[i] = get_pooled_int(handle_after - handle_before) alloc_before = alloc_after rc_before = rc_after fd_before = fd_after + handle_before = handle_after print(file=sys.stderr) # These checkers return False on success, True on failure @@ -102,7 +111,8 @@ def check_fd_deltas(deltas): for deltas, item_name, checker in [ (rc_deltas, 'references', check_rc_deltas), (alloc_deltas, 'memory blocks', check_rc_deltas), - (fd_deltas, 'file descriptors', check_fd_deltas) + (fd_deltas, 'file descriptors', check_fd_deltas), + (handle_deltas, 'handles', check_fd_deltas), ]: # ignore warmup runs deltas = deltas[nwarmup:] @@ -154,7 +164,7 @@ def dash_R_cleanup(fs, ps, pic, zdc, abcs): func1 = sys.getallocatedblocks func2 = sys.gettotalrefcount gc.collect() - return func1(), func2(), support.fd_count() + return func1(), func2(), support.fd_count(), handle_count() def clear_caches(): diff --git a/Lib/test/test_regrtest.py b/Lib/test/test_regrtest.py index af332ad15d922a..34ec86738bcbee 100644 --- a/Lib/test/test_regrtest.py +++ b/Lib/test/test_regrtest.py @@ -857,6 +857,28 @@ def test_leak(self): """) self.check_leak(code, 'file descriptors') + @unittest.skipUnless(sys.platform == 'win32', 'specific to Windows') + @unittest.skipUnless(Py_DEBUG, 'need a debug build') + def test_huntrleaks_handle_leak(self): + # test --huntrleaks for file descriptor leak + code = textwrap.dedent(""" + import os + import unittest + import _winapi + + process = _winapi.GetCurrentProcess() + handle = process + + class FDLeakTest(unittest.TestCase): + def test_leak(self): + handle2 = _winapi.DuplicateHandle( + process, handle, process, + 0, 1, + _winapi.DUPLICATE_SAME_ACCESS) + # bug: never close handle2 + """) + self.check_leak(code, 'handles') + def test_list_tests(self): # test --list-tests tests = [self.create_test() for i in range(5)] diff --git a/Misc/NEWS.d/next/Tests/2018-06-20-16-55-12.bpo-18174.343gmT.rst b/Misc/NEWS.d/next/Tests/2018-06-20-16-55-12.bpo-18174.343gmT.rst new file mode 100644 index 00000000000000..72b5feb1154f63 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2018-06-20-16-55-12.bpo-18174.343gmT.rst @@ -0,0 +1,2 @@ +regrtest now also checks for leak of Windows handles. Initial patch written +by Richard Oudkerk. diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 75d1f0678effe6..131fc269d022ba 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1229,6 +1229,33 @@ _winapi_GetModuleFileName_impl(PyObject *module, HMODULE module_handle) return PyUnicode_FromWideChar(filename, wcslen(filename)); } +/*[clinic input] +_winapi.GetProcessHandleCount + + ProcessHandle: HANDLE(c_default="GetCurrentProcess()") = NULL + / + +Return the number of open handles for the specified process. + +Return the number of open handles for the process specified +by ProcessHandle. If ProcessHandle is not given then the +handle count for the current process is given. + +[clinic start generated code]*/ + +static PyObject * +_winapi_GetProcessHandleCount_impl(PyObject *module, HANDLE ProcessHandle) +/*[clinic end generated code: output=af58910c23922016 input=089cb7546e598a2d]*/ +{ + DWORD HandleCount; + + if (!GetProcessHandleCount(ProcessHandle, &HandleCount)) + return PyErr_SetFromWindowsErr(0); + + return PyLong_FromUnsignedLong(HandleCount); +} + + /*[clinic input] _winapi.GetStdHandle -> HANDLE @@ -1714,6 +1741,7 @@ static PyMethodDef winapi_functions[] = { _WINAPI_GETEXITCODEPROCESS_METHODDEF _WINAPI_GETLASTERROR_METHODDEF _WINAPI_GETMODULEFILENAME_METHODDEF + _WINAPI_GETPROCESSHANDLECOUNT_METHODDEF _WINAPI_GETSTDHANDLE_METHODDEF _WINAPI_GETVERSION_METHODDEF _WINAPI_OPENPROCESS_METHODDEF diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index b14f087732ef43..ba730cc71e64bb 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -530,6 +530,38 @@ _winapi_GetModuleFileName(PyObject *module, PyObject *arg) return return_value; } +PyDoc_STRVAR(_winapi_GetProcessHandleCount__doc__, +"GetProcessHandleCount($module, ProcessHandle=None, /)\n" +"--\n" +"\n" +"Return the number of open handles for the specified process.\n" +"\n" +"Return the number of open handles for the process specified\n" +"by ProcessHandle. If ProcessHandle is not given then the\n" +"handle count for the current process is given."); + +#define _WINAPI_GETPROCESSHANDLECOUNT_METHODDEF \ + {"GetProcessHandleCount", (PyCFunction)_winapi_GetProcessHandleCount, METH_FASTCALL, _winapi_GetProcessHandleCount__doc__}, + +static PyObject * +_winapi_GetProcessHandleCount_impl(PyObject *module, HANDLE ProcessHandle); + +static PyObject * +_winapi_GetProcessHandleCount(PyObject *module, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + HANDLE ProcessHandle = GetCurrentProcess(); + + if (!_PyArg_ParseStack(args, nargs, "|" F_HANDLE ":GetProcessHandleCount", + &ProcessHandle)) { + goto exit; + } + return_value = _winapi_GetProcessHandleCount_impl(module, ProcessHandle); + +exit: + return return_value; +} + PyDoc_STRVAR(_winapi_GetStdHandle__doc__, "GetStdHandle($module, std_handle, /)\n" "--\n" @@ -941,4 +973,4 @@ _winapi_GetFileType(PyObject *module, PyObject *const *args, Py_ssize_t nargs, P exit: return return_value; } -/*[clinic end generated code: output=ec7f36eb7efc9d00 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=5f1f0b2da6249e90 input=a9049054013a1b77]*/ From e37995b8364dc7f02031b69b3ead487045439930 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 20 Jun 2018 17:10:31 +0200 Subject: [PATCH 2/2] Make regrtest less strict Detect a leak only if all runs leaked at least 1 handle to avoid false positives. --- Lib/test/libregrtest/refleak.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/libregrtest/refleak.py b/Lib/test/libregrtest/refleak.py index 901334584da6c1..3f8f16eb35b9b1 100644 --- a/Lib/test/libregrtest/refleak.py +++ b/Lib/test/libregrtest/refleak.py @@ -112,7 +112,7 @@ def check_fd_deltas(deltas): (rc_deltas, 'references', check_rc_deltas), (alloc_deltas, 'memory blocks', check_rc_deltas), (fd_deltas, 'file descriptors', check_fd_deltas), - (handle_deltas, 'handles', check_fd_deltas), + (handle_deltas, 'handles', check_rc_deltas), ]: # ignore warmup runs deltas = deltas[nwarmup:]