New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test failures with COUNT_ALLOCS #63726
Comments
When Python is compiled with COUNT_ALLOCS, some tests in test_gc and test_module fail. I'm attaching the patch that skips 3 of them and modifies assertions in one of them, so that the tests pass. I'm however still unsure about one of the skipped tests, since I'm unsure whether I totally understand what's wrong there - test_gc_ordinary_module_at_shutdown. My guess is that due to COUNT_ALLOCS causing immortal types, the "final_a" and "final_b" types don't get destroyed on line [1] as they do in builds without COUNT_ALLOCS. AFAICS they are only "un-immortalized" on this line and destroyed during the following loop [2]. The problem here is that the order of destroyed modules is not deterministic, so sometimes the builtins module gets destroyed before the "final_X" and there is no "print" function, which makes the __del__ functions from "final_X" fail. IMO the best thing to do is to just skip this test with COUNT_ALLOCS. But I may be wrong, I don't have a great insight into Python's GC and module unloading. [1] http://hg.python.org/cpython/annotate/0f48843652b1/Python/import.c#l383 |
And the patch... |
Since 3.4.0.b2, this also causes failures in another tests: test_io, test_logging, test_threading, test_warnings. There are various cases testing that some types get collected when the interpreter shuts down. I'm attaching a new patch that covers all of these. |
What is COUNT_ALLOCS? Python 3.4 now has sys.getallocatedblocks() and a new tracemalloc module which are compiled by default. |
As noted in Misc/SpecialBuilds: COUNT_ALLOCS Each type object grows three new members: /* Number of times an object of this type was allocated. */
int tp_allocs;
/* Number of times an object of this type was deallocated. */
int tp_frees;
/* Highwater mark: the maximum value of tp_allocs - tp_frees so
* far; or, IOW, the largest number of objects of this type alive at
* the same time.
*/
int tp_maxalloc; ... |
Why do you check hasattr(sys, 'getrefcount') in test_io.py, but hasattr(sys, 'getcounts') in all other tests? |
Good catch, using getrefcount was a mistake. I'm attaching a new version which always checks for getcounts (and also applies on 3.4.2). |
LGTM (only one nitpick -- there are trailing spaces in test_gc). But there are other tests which are failed with COUNT_ALLOCS. Here is a patch with additional fixes for these tests in test_gc and test_warnings. |
Thenks Antoine for great idea proposed in comments on Rietveld. Following patch introduces strip_python_stdout() which strips COUNT_ALLOCS debug output from stdout (unfortunately this operation is not always unambiguous) and call it in assert_python_ok() and assert_python_failure(). This automatically fixes a large number of tests. Also fixed a number of other tests failing with COUNT_ALLOCS. Virtually all tests are now fixed except test_doctest and test_distutils. A large part of the patch will be applied only to 3.4 and 2.7. With resolved bpo-23034 the patch for 3.5 will be much simpler, strip_python_stdout() and @requires_clean_stdout will gone away. |
With Python-3.5 and COUNT_ALLOCS enabled following new tests fail also: FAIL: test_is_finalizing (test.test_sys.SysModuleTest) Traceback (most recent call last):
File "/builddir/build/BUILD/Python-3.5.0/Lib/test/test_sys.py", line 767, in test_is_finalizing
self.assertEqual(stdout.rstrip(), b'True')
AssertionError: b'' != b'True'
---------------------------------
====================================================================== Traceback (most recent call last):
File "/builddir/build/BUILD/Python-3.5.0/Lib/test/test_traceback.py", line 210, in test_print_traceback_at_exit
self.assertEqual(stderr.splitlines(), expected)
AssertionError: Lists differ: [] != [b'Traceback (most recent call last):', b'[75 chars]ero']
Second list contains 3 additional elements.
First extra element 0:
b'Traceback (most recent call last):'
- []
+ [b'Traceback (most recent call last):',
+ b' File "<string>", line 8, in __init__',
+ b'ZeroDivisionError: division by zero'] |
COUNT_ALLOCS was added 22 years ago. I guess that the usecase is to track memory leaks, right? branch: legacy-trunk
|
00141-fix-tests_with_COUNT_ALLOCS-v5.patch: please don't do that! It makes tests much more verbose for a compilation option which is hidden and probably almost never used in the wild. The option has no configuration option for example. *If* you really want to keep the feature, I would prefer to make it more visible (add a configuration option) and disable the output at exit by default. It's better to add a new "-X showcountallocs" as it was done with "-X showrefcount". Before, we had to fix a lot of unit tests (as 00141-fix-tests_with_COUNT_ALLOCS-v5.patch) to strip the "[xxx refs]" from stderr, it was very annoying. "Python 3.4 now has sys.getallocatedblocks() and a new tracemalloc module which are compiled by default." IMHO these two debug features superseded COUNT_ALLOCS. Please try to convince me of the use case of this very old debug feature. |
FYI There is also bpo-23034 where is proposed "-X showalloccount" option to suppress the output, we use (custom patch) environment variable to filter out the verbose output in Fedora. |
Yes, I also don't want to use 00141-fix-tests_with_COUNT_ALLOCS-v5.patch if there is better alternative. See bpo-23034 (I'm uncertain only in option name). |
I no longer use this feature and I think that noone does. As you said, it seems to be obsoleted by other new features, so my vote would be to remove COUNT_ALLOCS altogether. |
I propose to emit a compiler warning (or even an error?) in 3.5.x and drop |
New changeset 5abf6cdcac4d by Serhiy Storchaka in branch '3.5': New changeset e7d84ecdd37d by Serhiy Storchaka in branch 'default': |
All tests now are passed in 3.6 on Linux. Making them passing in 3.5 requires too much changes that are not needed in 3.6. I don't think we need to pollute tests with these temporary workarounds. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: