The newly added `test_huntrleaks` test is failing on Python 2.7.14 debug build with COUNT_ALLOCS.


======================================================================
FAIL: test_huntrleaks (test.test_regrtest.ArgsTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/Python-2.7.14/Lib/test/test_regrtest.py", line 511, in test_huntrleaks
    self.check_leak(code, 'references')
  File "/builddir/build/BUILD/Python-2.7.14/Lib/test/test_regrtest.py", line 489, in check_leak
    self.assertIn(line2, output)
AssertionError: 'test_regrtest_huntrleaks leaked [1, 1, 1] references, sum=3\n' not found in 'Run tests sequentially\n0:00:00 load avg: 0.63 [1/1] test_regrtest_huntrleaks\nbeginning 6 repetitions\n123456\n......\ntest_regrtest_huntrleaks leaked [93, 93, 93] references, sum=279\n1 test failed:\n    test_regrtest_huntrleaks\n\nTotal duration: 32 ms\nTests result: FAILURE\n[53092 refs]\n'
----------------------------------------------------------------------


On Python 2.7.14 debug build *without* COUNT_ALLOCS the test passes fine, therefore I am not sure if there is a leak detected, or is it just COUNT_ALLOCS messing with the test and it should be skipped.

test_regrtest is not the single test failing when COUNT_ALLOCS is defined. Not least than 12 tests are failing.

In Python 3, many tests were fixed by skipping them if COUNT_ALLOCS is defined: commit a793037d803abf098d172f686e2b95d27863c54d of bpo-19527.

Since Python 3.6, even if COUNT_ALLOCS is defined, statistics are no more dumped by default, but only if -X showalloccount is defined: commit 7e160ce356036bccda2608d9ee10bfe276dfa97a of bpo-23034. Moreover, statitics are now written into stderr rather than stdout.

Commit a793037d803abf098d172f686e2b95d27863c54d is not related to this issue. It skips the tests that were failed due to other side effect of COUNT_ALLOCS -- it makes type immortal. This is not a problem in 2.7 since all types are immortal in 2.7.

But there is other patch in issue19527: 00141-fix-tests_with_COUNT_ALLOCS-v5.patch. It was not committed since issue23034 provided better solution. You can try to backport it to 2.7, but this will just complicate the testing code for almost no benefit. Nobody wanted to do this. It was a decision to ignore this issue in Python 3.5 and earlier.

I wrote PR 3910 to modify COUNT_ALLOCS: alllcations statistics are now written into stderr, rather than stdout.

The PR contains also changes to fix tests with COUNT_ALLOCS. I'm not sure about this part of the PR. I would prefer to not dump statistics by default, and add a new option to enable it.

I don't know if we can easily implement "-X showalloccount", since Python 2.7 doesn't have sys._xoptions. Maybe we could use a new environment variable instead: PYTHONSHOWALLOCCOUNT=1?

I would prefer to add a new option instead of having to patch so many unit tests:

haypo@selma$ git diff 2.7.. --stat
 Lib/json/tests/test_tool.py                                                 |  8 ++++++--
 Lib/test/support/__init__.py                                                |  3 +++
 Lib/test/test_abc.py                                                        |  2 ++
 Lib/test/test_gc.py                                                         |  4 +++-
 Lib/test/test_hash.py                                                       |  2 +-
 Lib/test/test_module.py                                                     |  3 ++-
 Lib/test/test_multiprocessing.py                                            |  4 +++-
 Lib/test/test_regrtest.py                                                   |  1 +
 Lib/test/test_subprocess.py                                                 | 17 +++++++++++++++++
 Lib/test/test_sys.py                                                        | 21 +++++++++++++++------
 Lib/test/test_threading.py                                                  | 11 ++++++++---
 Lib/test/test_tools.py                                                      | 10 +++++++---
 Lib/test/test_warnings.py                                                   |  4 ++++
 Lib/test/test_weakref.py                                                    |  1 +
 Misc/NEWS.d/next/Core and Builtins/2017-10-06-21-56-47.bpo-31692.osJuVJ.rst |  2 ++
 Python/pythonrun.c                                                          |  2 +-
 16 files changed, 76 insertions(+), 19 deletions(-)

Do you going to backport also -X showrefcount?

My colleague Iryna Shcherbina came to me with this issue. I'm not sure that we really need to support COUNT_ALLOCS. But strangely, it seems simpler to fix bugs rather than guessing if users like this debug mode or not :-)

There are different ways to fix tests to make them passing with COUNT_ALLOCS. As I wrote, I would prefer to add a new Python 2.7 option to explicitly asks to dump allocations statistics: add a new PYTHONSHOWALLOCCOUNT=1 environment variable for example.

What do you think Serhiy? Do you prefer to decorate many 2.7 tests to skip them if COUNT_ALLOCS is used, or to add a new option?

I should try to implement -X showalloccount :-)

> I don't know if we can easily implement "-X showalloccount", since
> Python 2.7 doesn't have sys._xoptions. Maybe we could use a new
> environment variable instead: PYTHONSHOWALLOCCOUNT=1?


That is how it is bypassed in Fedora Py2 builds currently. I am attaching the patch we use.

I left this on to Benjamin, the RM of Python 2.7.

Serhiy:
> Do you going to backport also -X showrefcount?

I see -X as a Python3 only thing. If I have to choose, I would prefer to add a new environment variable, since it's more in the "Python2 style".

Iryna:
> That is how it is bypassed in Fedora Py2 builds currently. I am attaching the patch we use.

Oh, nice to see that we get the same idea: opt-in environment variable.

I'm unhappy with the variable name: PYTHONDUMPCOUNTS seems too generic IMHO :-(

I prefer "PYTHONSHOWALLOCCOUNT" since it's closer to Python 3 "-X showalloccount" option, and "alloc" mentions that they are counters on allocations. Not just "random" counters.

Ok, so. I wrote a different PR: PR 3927 adds a new PYTHONSHOWALLOCCOUNT environment variable and write allocations statistics into stderr (rather than stdout) when PYTHONSHOWALLOCCOUNT is set.

The PR also fixes the test suite for COUNT_ALLOCS.

The PR 3927 is much shorter because PYTHONSHOWALLOCCOUNT now has to be defined to dump allocations statistics.

haypo@selma$ git diff 2.7.. --stat
 Doc/using/cmdline.rst                                                       | 7 +++++++
 Lib/test/support/__init__.py                                                | 3 +++
 Lib/test/test_abc.py                                                        | 1 +
 Lib/test/test_gc.py                                                         | 4 +++-
 Lib/test/test_regrtest.py                                                   | 1 +
 Lib/test/test_sys.py                                                        | 5 ++++-
 Lib/test/test_weakref.py                                                    | 1 +
 Misc/NEWS.d/next/Core and Builtins/2017-10-09-11-03-13.bpo-31692.5-bpdk.rst | 4 ++++
 Python/pythonrun.c                                                          | 4 +++-
 9 files changed, 27 insertions(+), 3 deletions(-)

I tested PR 3927. I run "./python -m test -j8 -r" with PYTHONSHOWALLOCCOUNT unset: the full Python test suite pass in release and debug modes:

* ./configure CFLAGS="-D COUNT_ALLOCS=1"
* ./configure  --with-pydebug CFLAGS="-D COUNT_ALLOCS=1"

I have run a test build with the patch from PR 3927 applied, and all tests passed.
Full build log can be viewed here: https://kojipkgs.fedoraproject.org//work/tasks/8392/22348392/build.log

Adding PYTHONSHOWALLOCCOUNT looks rather like a new feature to me. It is on to Bejaming to make a decision about this.

The difference between an environment variable and a command line option is that the former is inherited by children, and the latter is not (this can be considered a drawback or an advantage).

COUNT_ALLOCS is not the only option that spoils Python output. Py_TRACE_REFS adds even more noise to stderr, and this is a default option in debug build. How many tests are still left broken if just change the COUNT_ALLOCS output to stderr instead of stdout?

Serhiy: "Do you going to backport also -X showrefcount?"

I just created bpo-31733 "Add PYTHONSHOWREFCOUNT environment variable to Python 2.7".

> How many tests are still left broken if just change the COUNT_ALLOCS output to stderr instead of stdout?

That's my first PR: PR 3910.


> COUNT_ALLOCS is not the only option that spoils Python output. Py_TRACE_REFS adds even more noise to stderr, and this is a default option in debug build.

Again, I created bpo-31733 to propose to make this *super annoying* [xxx refs] line *by default*. It makes this issue more consistent ;-)

(I prefer to not split the discussion on this issue and my 2 PR, I prefer to only discuss the design in this issue.)

https://github.com/python/cpython/pull/3927#pullrequestreview-67963845

Serhiy:
> Using the requires_type_collecting decorator LGTM. But I don't sure about adding PYTHONSHOWALLOCCOUNT. This looks like a new feature to me.
> What if split this PR on two parts? The one fixes issues related to immortal types, the other is about additional output.

Both changes are connected. If you only want to fix tests, that's the purpose of my much longer PR 3910.

"This looks like a new feature to me."

My PR changes the default behaviour, but COUNT_ALLOCS is not enabled by default, not even by the debug mode. You have to explicitely enable it. I expect that developers who use this option are able to track the Python 2.7 changelog and "discover" the newly added environment variable by themself. Moreover, defining PYTHONSHOWALLOCCOUNT=1 is "backward compatible" with Python < 2.7.15 and Python 3, since unknown environment variables are just ignored.

All Victor's PRs LGTM is they look good to Benjamin.

But on the next iteration we can get a report that tests don't work if set PYTHONSHOWREFCOUNT.

New changeset 7b4ba62e388474e811268322b47f80d464933541 by Victor Stinner in branch '2.7':
[2.7] bpo-31692: Add PYTHONSHOWALLOCCOUNT env var (GH-3927)
https://github.com/python/cpython/commit/7b4ba62e388474e811268322b47f80d464933541

Python 2.7.15 will require to compile Python with COUNT_ALLOCS defined *and* to set the PYTHONSHOWALLOCCOUNT environment variable (ex: PYTHONSHOWALLOCCOUNT=1) to dump allocation counts. Moreover, counts are now dumped into stderr, instead of stdout.

Python 2.7 test suite now pass when Python is compiled with COUNT_ALLOCS.

Python 2.7 test suite doens't pass with PYTHONSHOWALLOCCOUNT set. I don't think that it's worth it to fix this case.

Isn't worth to document these changes in What's New?

Serhiy: "Isn't worth to document these changes in What's New?"

Ok, I created the PR 4019.

New changeset 355393e7438deeab4aeec3fcffea65e8cada083b by Victor Stinner in branch '2.7':
[2.7] bpo-31733, bpo-31692: Document 2 new env vars in What's New in Python 2.7 (GH-4019)
https://github.com/python/cpython/commit/355393e7438deeab4aeec3fcffea65e8cada083b

I documented the two new environment variables in What's New in Python 2.7, so this issue can now be closed again.