diff --git a/.flake8 b/.flake8 index 6581552aa..f06f8344a 100644 --- a/.flake8 +++ b/.flake8 @@ -3,6 +3,11 @@ # T001 = print() statement [flake8] +ignore = + # ambiguous variable name 'l' + E741, + # line break after binary operator + W504 per-file-ignores = setup.py:T001 scripts/*:T001 diff --git a/HISTORY.rst b/HISTORY.rst index 24186c3d2..c99b9c4e5 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -24,6 +24,7 @@ XXXX-XX-XX >>> proc psutil.Process(pid=12739, name='python3', status='terminated', exitcode=, started='15:08:20') +- 1757_: memory leak tests are now stable. **Bug fixes** diff --git a/appveyor.yml b/appveyor.yml index d22c1cb4f..e0912a1ed 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -69,6 +69,7 @@ build: off test_script: - "%WITH_COMPILER% %PYTHON%/python.exe scripts/internal/winmake.py test" + - "%WITH_COMPILER% %PYTHON%/python.exe scripts/internal/winmake.py test-memleaks" after_test: - "%WITH_COMPILER% %PYTHON%/python.exe scripts/internal/winmake.py wheel" diff --git a/docs/conf.py b/docs/conf.py index b056d20fe..f4a32b987 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -267,21 +267,21 @@ def get_version(): # -- Options for LaTeX output --------------------------------------------- latex_elements = { - # The paper size ('letterpaper' or 'a4paper'). - # - # 'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + # + # 'papersize': 'letterpaper', - # The font size ('10pt', '11pt' or '12pt'). - # - # 'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + # + # 'pointsize': '10pt', - # Additional stuff for the LaTeX preamble. - # - # 'preamble': '', + # Additional stuff for the LaTeX preamble. + # + # 'preamble': '', - # Latex figure (float) alignment - # - # 'figure_align': 'htbp', + # Latex figure (float) alignment + # + # 'figure_align': 'htbp', } # Grouping the document tree into LaTeX files. List of tuples diff --git a/psutil/_compat.py b/psutil/_compat.py index 145fb71d4..17f38485e 100644 --- a/psutil/_compat.py +++ b/psutil/_compat.py @@ -8,7 +8,6 @@ """ import collections -import contextlib import errno import functools import os @@ -23,7 +22,7 @@ # literals "u", "b", # collections module - "lru_cache", "redirect_stderr", + "lru_cache", # shutil module "which", "get_terminal_size", # python 3 exceptions @@ -423,17 +422,3 @@ def get_terminal_size(fallback=(80, 24)): return (res[1], res[0]) except Exception: return fallback - - -# python 3.4 -try: - from contextlib import redirect_stderr -except ImportError: - @contextlib.contextmanager - def redirect_stderr(target): - original = sys.stderr - try: - sys.stderr = target - yield - finally: - sys.stderr = original diff --git a/psutil/arch/windows/net.c b/psutil/arch/windows/net.c index 0891bdba4..8d8f7d1c0 100644 --- a/psutil/arch/windows/net.c +++ b/psutil/arch/windows/net.c @@ -16,42 +16,35 @@ static PIP_ADAPTER_ADDRESSES -psutil_get_nic_addresses() { - // Note: GetAdaptersAddresses() increase the handle count on first - // call (and not anymore later on). - // allocate a 15 KB buffer to start with - int outBufLen = 15000; - DWORD dwRetVal = 0; - ULONG attempts = 0; - PIP_ADAPTER_ADDRESSES pAddresses = NULL; - - do { - pAddresses = (IP_ADAPTER_ADDRESSES *) malloc(outBufLen); - if (pAddresses == NULL) { - PyErr_NoMemory(); - return NULL; - } - - dwRetVal = GetAdaptersAddresses(AF_UNSPEC, 0, NULL, pAddresses, - &outBufLen); - if (dwRetVal == ERROR_BUFFER_OVERFLOW) { - free(pAddresses); - pAddresses = NULL; - } - else { - break; - } - - attempts++; - } while ((dwRetVal == ERROR_BUFFER_OVERFLOW) && (attempts < 3)); +psutil_get_nic_addresses(void) { + ULONG bufferLength = 0; + PIP_ADAPTER_ADDRESSES buffer; + + if (GetAdaptersAddresses(AF_UNSPEC, 0, NULL, NULL, &bufferLength) + != ERROR_BUFFER_OVERFLOW) + { + PyErr_SetString(PyExc_RuntimeError, + "GetAdaptersAddresses() syscall failed."); + return NULL; + } - if (dwRetVal != NO_ERROR) { - PyErr_SetString( - PyExc_RuntimeError, "GetAdaptersAddresses() syscall failed."); + buffer = malloc(bufferLength); + if (buffer == NULL) { + PyErr_NoMemory(); + return NULL; + } + memset(buffer, 0, bufferLength); + + if (GetAdaptersAddresses(AF_UNSPEC, 0, NULL, buffer, &bufferLength) + != ERROR_SUCCESS) + { + free(buffer); + PyErr_SetString(PyExc_RuntimeError, + "GetAdaptersAddresses() syscall failed."); return NULL; } - return pAddresses; + return buffer; } @@ -191,11 +184,11 @@ psutil_net_if_addrs(PyObject *self, PyObject *args) { for (i = 0; i < (int) pCurrAddresses->PhysicalAddressLength; i++) { if (i == (pCurrAddresses->PhysicalAddressLength - 1)) { sprintf_s(ptr, _countof(buff_macaddr), "%.2X\n", - (int)pCurrAddresses->PhysicalAddress[i]); + (int)pCurrAddresses->PhysicalAddress[i]); } else { sprintf_s(ptr, _countof(buff_macaddr), "%.2X-", - (int)pCurrAddresses->PhysicalAddress[i]); + (int)pCurrAddresses->PhysicalAddress[i]); } ptr += 3; } @@ -325,8 +318,7 @@ psutil_net_if_addrs(PyObject *self, PyObject *args) { /* * Provides stats about NIC interfaces installed on the system. - * TODO: get 'duplex' (currently it's hard coded to '2', aka - 'full duplex') + * TODO: get 'duplex' (currently it's hard coded to '2', aka 'full duplex') */ PyObject * psutil_net_if_stats(PyObject *self, PyObject *args) { diff --git a/psutil/tests/__init__.py b/psutil/tests/__init__.py index 41a791c1b..b452775f7 100644 --- a/psutil/tests/__init__.py +++ b/psutil/tests/__init__.py @@ -476,12 +476,16 @@ def terminate(proc_or_pid, sig=signal.SIGTERM, wait_timeout=GLOBAL_TIMEOUT): from psutil._psposix import wait_pid def wait(proc, timeout): - try: - return psutil.Process(proc.pid).wait(timeout) - except psutil.NoSuchProcess: - # Needed to kill zombies. - if POSIX: - return wait_pid(proc.pid, timeout) + if isinstance(proc, subprocess.Popen) and not PY3: + proc.wait() + else: + proc.wait(timeout) + if WINDOWS and isinstance(proc, subprocess.Popen): + # Otherwise PID may still hang around. + try: + return psutil.Process(proc.pid).wait(timeout) + except psutil.NoSuchProcess: + pass def sendsig(proc, sig): # If the process received SIGSTOP, SIGCONT is necessary first, @@ -899,22 +903,15 @@ class TestMemoryLeak(PsutilTestCase): typically functions implemented in C which forgot to free() memory from the heap. It does so by checking whether the process memory usage increased before and after calling the function many times. - The logic: - - call_fun_n_times() - if mem_diff > tolerance: - call_fun_for_3_secs() - if mem_diff > 0: - return 1 # failure - return 0 # success Note that this is hard (probably impossible) to do reliably, due to how the OS handles memory, the GC and so on (memory can even - decrease!). In order to avoid false positives you should adjust the - tolerance of each individual test case, but most of the times you - won't have to. + decrease!). In order to avoid false positives, in case of failure + (mem > 0) we retry the test for up to 5 times, increasing call + repetitions each time. If the memory keeps increasing then it's a + failure. - If available (Linux, OSX, Windows) USS memory is used for comparison, + If available (Linux, OSX, Windows), USS memory is used for comparison, since it's supposed to be more precise, see: http://grodola.blogspot.com/2016/02/psutil-4-real-process-memory-and-environ.html If not, RSS memory is used. mallinfo() on Linux and _heapwalk() on @@ -932,15 +929,15 @@ def test_fun(self): self.execute(some_function) """ # Configurable class attrs. - times = 1000 + times = 200 warmup_times = 10 - tolerance = 4096 # memory - retry_for = 3.0 # seconds + retries = 5 + tolerance = 0 # memory verbose = True - def setUp(self): - self._thisproc = psutil.Process() - gc.collect() + @classmethod + def setUpClass(cls): + cls._thisproc = psutil.Process() def _get_mem(self): # USS is the closest thing we have to "real" memory usage and it @@ -957,77 +954,63 @@ def _get_fds_or_handles(self): def _call(self, fun): return fun() - def _itercall(self, fun, iterator): + def _call_ntimes(self, fun, times): """Get 2 distinct memory samples, before and after having called fun repeadetly, and return the memory difference. """ - ncalls = 0 - gc.collect() + gc.collect(generation=1) mem1 = self._get_mem() - for x in iterator: + for x in range(times): ret = self._call(fun) - ncalls += 1 del x, ret - gc.collect() + gc.collect(generation=1) mem2 = self._get_mem() self.assertEqual(gc.garbage, []) - diff = mem2 - mem1 - if diff < 0: - self._log("negative memory diff -%s" % (bytes2human(abs(diff)))) - return (diff, ncalls) - - def _call_ntimes(self, fun, times): - return self._itercall(fun, range(times))[0] - - def _call_for(self, fun, secs): - def iterator(secs): - stop_at = time.time() + secs - while time.time() < stop_at: - yield - return self._itercall(fun, iterator(secs)) + diff = mem2 - mem1 # can also be negative + return diff def _log(self, msg): if self.verbose: print_color(msg, color="yellow", file=sys.stderr) - def execute(self, fun, times=times, warmup_times=warmup_times, - tolerance=tolerance, retry_for=retry_for): + def execute(self, fun, times=None, warmup_times=None, retries=None, + tolerance=None): """Test a callable.""" - if times <= 0: - raise ValueError("times must be > 0") - if warmup_times < 0: - raise ValueError("warmup_times must be >= 0") - if tolerance is not None and tolerance < 0: - raise ValueError("tolerance must be >= 0") - if retry_for is not None and retry_for < 0: - raise ValueError("retry_for must be >= 0") + times = times if times is not None else self.times + warmup_times = warmup_times if warmup_times is not None \ + else self.warmup_times + retries = retries if retries is not None else self.retries + tolerance = tolerance if tolerance is not None else self.tolerance + try: + assert times >= 1, "times must be >= 1" + assert warmup_times >= 0, "warmup_times must be >= 0" + assert retries >= 0, "retries must be >= 0" + assert tolerance >= 0, "tolerance must be >= 0" + except AssertionError as err: + raise ValueError(str(err)) # warm up self._call_ntimes(fun, warmup_times) - mem1 = self._call_ntimes(fun, times) - - if mem1 > tolerance: - # This doesn't necessarily mean we have a leak yet. - # At this point we assume that after having called the - # function so many times the memory usage is stabilized - # and if there are no leaks it should not increase - # anymore. Let's keep calling fun for N more seconds and - # fail if we notice any difference (ignore tolerance). - msg = "+%s after %s calls; try calling fun for another %s secs" % ( - bytes2human(mem1), times, retry_for) - if not retry_for: - raise self.fail(msg) + messages = [] + prev_mem = 0 + increase = times + for idx in range(1, retries + 1): + mem = self._call_ntimes(fun, times) + msg = "Run #%s: extra-mem=%s, per-call=%s, calls=%s" % ( + idx, bytes2human(mem), bytes2human(mem / times), times) + messages.append(msg) + success = mem <= tolerance or mem < prev_mem + if success: + if idx > 1: + self._log(msg) + return else: + if idx == 1: + print() # NOQA self._log(msg) - - mem2, ncalls = self._call_for(fun, retry_for) - if mem2 > mem1: - # failure - msg = "+%s memory increase after %s calls; " % ( - bytes2human(mem1), times) - msg += "+%s after another %s calls over %s secs" % ( - bytes2human(mem2), ncalls, retry_for) - raise self.fail(msg) + times += increase + prev_mem = mem + raise self.fail(". ".join(messages)) def execute_w_exc(self, exc, fun, **kwargs): """Convenience method to test a callable while making sure it diff --git a/psutil/tests/test_memory_leaks.py b/psutil/tests/test_memory_leaks.py index 02e61aa00..0bf2ad8d5 100755 --- a/psutil/tests/test_memory_leaks.py +++ b/psutil/tests/test_memory_leaks.py @@ -21,7 +21,6 @@ import psutil import psutil._common -from psutil import FREEBSD from psutil import LINUX from psutil import MACOS from psutil import OPENBSD @@ -30,7 +29,6 @@ from psutil import WINDOWS from psutil._compat import ProcessLookupError from psutil._compat import super -from psutil.tests import CIRRUS from psutil.tests import create_sockets from psutil.tests import get_testfn from psutil.tests import HAS_CPU_AFFINITY @@ -244,7 +242,7 @@ def test_connections(self): # be executed. with create_sockets(): kind = 'inet' if SUNOS else 'all' - self.execute(lambda: self.proc.connections(kind), times=100) + self.execute(lambda: self.proc.connections(kind)) @unittest.skipIf(not HAS_ENVIRON, "not supported") def test_environ(self): @@ -402,16 +400,9 @@ def test_pids(self): # --- net - # XXX - @unittest.skipIf(TRAVIS and MACOS, "false positive on TRAVIS + MACOS") - @unittest.skipIf(CIRRUS and FREEBSD, "false positive on CIRRUS + FREEBSD") @skip_if_linux() @unittest.skipIf(not HAS_NET_IO_COUNTERS, 'not supported') def test_net_io_counters(self): - if WINDOWS: - # GetAdaptersAddresses() increases the handle count on first - # call (only). - psutil.net_io_counters() self.execute(lambda: psutil.net_io_counters(nowrap=False)) @skip_if_linux() @@ -420,23 +411,15 @@ def test_net_connections(self): # always opens and handle on Windows() (once) psutil.net_connections(kind='all') with create_sockets(): - self.execute(lambda: psutil.net_connections(kind='all'), times=100) + self.execute(lambda: psutil.net_connections(kind='all')) def test_net_if_addrs(self): - if WINDOWS: - # GetAdaptersAddresses() increases the handle count on first - # call (only). - psutil.net_if_addrs() # Note: verified that on Windows this was a false positive. - self.execute(psutil.net_if_addrs, - tolerance=80 * 1024 if WINDOWS else 4096) + tolerance = 80 * 1024 if WINDOWS else self.tolerance + self.execute(psutil.net_if_addrs, tolerance=tolerance) - @unittest.skipIf(TRAVIS, "EPERM on travis") + # @unittest.skipIf(TRAVIS, "EPERM on travis") def test_net_if_stats(self): - if WINDOWS: - # GetAdaptersAddresses() increases the handle count on first - # call (only). - psutil.net_if_stats() self.execute(psutil.net_if_stats) # --- sensors @@ -462,7 +445,6 @@ def test_sensors_fans(self): def test_boot_time(self): self.execute(psutil.boot_time) - @unittest.skipIf(WINDOWS, "XXX produces a false positive on Windows") def test_users(self): self.execute(psutil.users) @@ -495,6 +477,8 @@ def test_win_service_get_description(self): class TestUnclosedFdsOrHandles(TestFdsLeak): + # Note: on Windows certain C functions increase the handles count + # on first call (and never again). def test_process_apis(self): p = psutil.Process() @@ -525,7 +509,7 @@ def test_system_apis(self): for fun, name in ns.iter(ns.all): if WINDOWS: fun() - if MACOS and name == 'connections': + if MACOS and name == 'net_connections': continue # raise AD self.execute(fun) diff --git a/psutil/tests/test_testutils.py b/psutil/tests/test_testutils.py index 64bd8d3b2..6c5100945 100755 --- a/psutil/tests/test_testutils.py +++ b/psutil/tests/test_testutils.py @@ -12,7 +12,6 @@ import collections import contextlib import errno -import io import os import socket import stat @@ -24,8 +23,6 @@ from psutil._common import open_binary from psutil._common import open_text from psutil._common import supports_ipv6 -from psutil._compat import PY3 -from psutil._compat import redirect_stderr from psutil.tests import bind_socket from psutil.tests import bind_unix_socket from psutil.tests import call_until @@ -40,7 +37,6 @@ from psutil.tests import PYTHON_EXE from psutil.tests import reap_children from psutil.tests import retry -from psutil.tests import retry_on_failure from psutil.tests import safe_mkdir from psutil.tests import safe_rmpath from psutil.tests import serialrun @@ -353,54 +349,31 @@ def test_create_sockets(self): @serialrun class TestMemLeakClass(TestMemoryLeak): - @retry_on_failure() def test_times(self): def fun(): cnt['cnt'] += 1 cnt = {'cnt': 0} - self.execute(fun, times=1, warmup_times=10) - self.assertEqual(cnt['cnt'], 11) - self.execute(fun, times=10, warmup_times=10) - self.assertEqual(cnt['cnt'], 31) - - @retry_on_failure() - def test_warmup_times(self): - def fun(): - cnt['cnt'] += 1 - cnt = {'cnt': 0} - self.execute(fun, times=1, warmup_times=10) - self.assertEqual(cnt['cnt'], 11) + self.execute(fun, times=10, warmup_times=15) + self.assertEqual(cnt['cnt'], 25) def test_param_err(self): self.assertRaises(ValueError, self.execute, lambda: 0, times=0) self.assertRaises(ValueError, self.execute, lambda: 0, times=-1) self.assertRaises(ValueError, self.execute, lambda: 0, warmup_times=-1) self.assertRaises(ValueError, self.execute, lambda: 0, tolerance=-1) - self.assertRaises(ValueError, self.execute, lambda: 0, retry_for=-1) + self.assertRaises(ValueError, self.execute, lambda: 0, retries=-1) - @retry_on_failure() def test_leak(self): - def fun(): - ls.append("x" * 24 * 1024) ls = [] - times = 100 - self.assertRaises(AssertionError, self.execute, fun, times=times, - warmup_times=10, retry_for=None) - self.assertEqual(len(ls), times + 10) - @retry_on_failure(retries=20) # 2 secs - def test_leak_with_retry(self, ls=[]): - def fun(): + def fun(ls=ls): ls.append("x" * 24 * 1024) - times = 100 - f = io.StringIO() if PY3 else io.BytesIO() - with redirect_stderr(f): - self.assertRaises(AssertionError, self.execute, fun, times=times, - retry_for=0.1) - self.assertIn("try calling fun for another", f.getvalue()) - self.assertGreater(len(ls), times) - - @retry_on_failure() + + try: + self.assertRaises(AssertionError, self.execute, fun) + finally: + del ls + def test_tolerance(self): def fun(): ls.append("x" * 24 * 1024) @@ -410,13 +383,10 @@ def fun(): tolerance=200 * 1024 * 1024) self.assertEqual(len(ls), times) - @retry_on_failure() def test_execute_w_exc(self): def fun(): 1 / 0 - # XXX: use high tolerance, occasional false positive - self.execute_w_exc(ZeroDivisionError, fun, times=2000, - warmup_times=20, tolerance=200 * 1024, retry_for=3) + self.execute_w_exc(ZeroDivisionError, fun) with self.assertRaises(ZeroDivisionError): self.execute_w_exc(OSError, fun) @@ -426,6 +396,7 @@ def fun(): self.execute_w_exc(ZeroDivisionError, fun) +@serialrun class TestFdsLeakClass(TestFdsLeak): def test_unclosed_files(self): diff --git a/scripts/internal/winmake.py b/scripts/internal/winmake.py index de99c3276..a83aed3e0 100755 --- a/scripts/internal/winmake.py +++ b/scripts/internal/winmake.py @@ -488,7 +488,6 @@ def test_failed(): def test_memleaks(): """Run memory leaks tests""" build() - test_setup() sh("%s psutil\\tests\\test_memory_leaks.py" % PYTHON)