[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[STINNER Victor]

STINNER Victor added the comment:

bpo-30536 has been marked as a duplicate of this issue: [EASY] 
SubinterpThreadingTests.test_threads_join_2() of test_threading leaks 
references.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[STINNER Victor]

STINNER Victor added the comment:

-SET_SYS_FROM_STRING_BORROW_INT_RESULT("warnoptions", warnoptions); 

Oh, it seems like the regression was introduced by me in the commit 
8fea252a507024edf00d5d98881d22dc8799a8d3, see:
http://bugs.python.org/issue18520#msg201472

Or maybe it comes from recent changes made by Eric Snow and Nick Coghlan 
related to Python initiallization (PEP 432). I don't know.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[Stéphane Wirtel]

Stéphane Wirtel added the comment:

I have pushed a PR, if you can check it. Thanks

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[Stéphane Wirtel]

Changes by Stéphane Wirtel :


--
pull_requests: +2059

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[STINNER Victor]

STINNER Victor added the comment:

Oh, 1abcf6700b4da6207fe859de40c6c1bada6b4fec introduced two more reference 
leaks.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[STINNER Victor]

STINNER Victor added the comment:

SubinterpreterTest.test_subinterps of test_capi also leaks. But it is likely 
the same bug than this issue (SubinterpreterTest.test_callbacks_leak() of 
test_atexit leaks).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[STINNER Victor]

STINNER Victor added the comment:

> At the end, you should get the commit 
> 6b4be195cd8868b76eb6fbe166acc39beee8ce36.

The commit is a giant change. So let me help you, the following change is 
strange. value is replaced whereas its value is non-NULL... Maybe it's the 
regression? ;-)

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 90f8551..03601ea 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -291,6 +291,9 @@ import_init(PyInterpreterState *interp, PyObject *sysmod)
 
 /* Install importlib as the implementation of import */
 value = PyObject_CallMethod(importlib, "_install", "OO", sysmod, impmod);
+if (value != NULL)
+value = PyObject_CallMethod(importlib,
+"_install_external_importers", "");
 if (value == NULL) {
 PyErr_Print();
 Py_FatalError("Py_Initialize: importlib install failed");


Stéphane Wirtel (matrixise): "this issue can be executed on Linux, I think I am 
going to work on this one."

Would you like to work on a patch?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[STINNER Victor]

STINNER Victor added the comment:

The failing unit test was added by:

commit 2d350fd8af29eada0c3f264a91df6ab4af4a05fd
Author: Antoine Pitrou 
Date:   Thu Aug 1 20:56:12 2013 +0200

Issue #18619: Fix atexit leaking callbacks registered from 
sub-interpreters, and make it GC-aware.


Using git bisect, I found that the leak was introduced by:

commit 6b4be195cd8868b76eb6fbe166acc39beee8ce36
Author: Eric Snow 
Date:   Mon May 22 21:36:03 2017 -0700

bpo-22257: Small changes for PEP 432. (#1728)

PEP 432 specifies a number of large changes to interpreter startup code, 
including exposing a cleaner C-API. The major changes depend on a number of 
smaller changes. This patch includes all those smaller changes.


To run a git bisection, start with an old commit, 1 month ago: 
5d7a8d0c13737fd531b722ad76c505ef47aac96a (May, 1). Spoiler: the test doesn't 
leak at this bisection.

git bisect reset
git bisect start

git checkout master
make && ./python -m test -R 3:3 -m test_callbacks_leak test_atexit
# test fails
git bisect bad  # bad=test fails (ref leak)

git checkout 5d7a8d0c13737fd531b722ad76c505ef47aac96a
make && ./python -m test -R 3:3 -m test_callbacks_leak test_atexit
# test pass
git bisect good  # good=test pass (no leak)

make && ./python -m test -R 3:3 -m test_callbacks_leak test_atexit
# git bisect good or bad depending on the test result

# ... continue until git bisect finds the commit ...

At the end, you should get the commit 6b4be195cd8868b76eb6fbe166acc39beee8ce36.


@Eric Snow: Please don't fix the bug, please explain how to fix it ;-)

--
nosy: +eric.snow

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[Stéphane Wirtel]

Stéphane Wirtel added the comment:

this issue can be executed on Linux, I think I am going to work on this one.

--
nosy: +matrixise

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30547] [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks references

[STINNER Victor]

STINNER Victor added the comment:

I tagged this issue as easy. I consider that it's a good exercice for new 
contributors. Core developers: please let new contributors try to fix it since 
this issue is not critical, try to explain how to fix it rather than writing 
the fix ;-)

--
keywords: +easy (C)
title: [Windows] SubinterpreterTest.test_callbacks_leak() of test_atexit leaks 
references -> [EASY][Windows] SubinterpreterTest.test_callbacks_leak() of 
test_atexit leaks references

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com