Title: Reference leaks introduced by bpo-30860
Type: resource usage Stage: resolved
Components: Versions: Python 3.7
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: eric.smith, eric.snow, ncoghlan, skrah, vstinner
Priority: normal Keywords: patch

Created on 2017-09-11 16:29 by vstinner, last changed 2017-09-14 07:36 by eric.snow. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3506 merged eric.snow, 2017-09-11 22:54
PR 3567 merged eric.snow, 2017-09-14 07:15
Messages (8)
msg301882 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-11 16:29
The following commands show memory leaks introduced by bpo-30860:

$ ./python -m test -R 3:3 test_atexit -m test.test_atexit.SubinterpreterTest.test_callbacks_leak

$ ./python -m test -R 3:3 test_atexit -m test.test_atexit.SubinterpreterTest.test_callbacks_leak_refcycle

$ ./python -m test -R 3:3 test_threading -m test.test_threading.SubinterpThreadingTests.test_threads_join

$ ./python -m test -R 3:3 test_capi -m test.test_capi.SubinterpreterTest.test_subinterps
msg301883 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-11 16:30
> The following commands show memory leaks introduced by bpo-30860:

Oh, I'm talking about the commit 2ebc5ce42a8a9e047e790aefbf9a94811569b2b6.
msg301886 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2017-09-11 16:36
Could be the same as #31408.
msg301889 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-11 17:14
The leak comes from the following lines of _PySys_EndInit():
    PyObject *warnoptions = get_warnoptions();
    if (warnoptions == NULL)
        return -1;
    SET_SYS_FROM_STRING_BORROW_INT_RESULT("warnoptions", warnoptions);

    PyObject *xoptions = get_xoptions();
    if (xoptions == NULL)
        return -1;
    SET_SYS_FROM_STRING_BORROW_INT_RESULT("_xoptions", xoptions);

It's not the first time that I have an issue with these attributes. The last reference weak caused by multiple interpreters was also related to this one if I recall correctly.

See bpo-30598 and my commit 865de27dd79571a4a5c7a7d22a07fb909c4a9f8e.
msg301891 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-11 17:16
Extract of my msg295399: "The problem is that warnoptions is stored in a C global variable *and* in sys.warnoptions of each interpreter. The ownership of this variable is unclear."

Maybe we need a change similar to bpo-28411 (commit 86b7afdfeee77993fe896a2aa13b3f4f95973f16) which removed the "modules" field from Py_InterpreterState.
msg301904 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2017-09-11 21:21
I'm looking into this.
msg301906 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2017-09-11 21:31
Moving warnoptions (and xoptions) out of PyInterpreterState seems like a good idea to me for the same reasons as applied to sys.modules.  The case for these two is even stronger since they are only used in sysmodule.c.
msg302154 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2017-09-14 07:36
New changeset dae0276bb6bc7281d59fb0b8f1aab31634ee80dc by Eric Snow in branch 'master':
bpo-30860: Fix a refleak. (#3567)
Date User Action Args
2017-09-14 07:36:00eric.snowsetmessages: + msg302154
2017-09-14 07:15:42eric.snowsetpull_requests: + pull_request3557
2017-09-12 11:39:13skrahlinkissue31408 superseder
2017-09-12 01:00:56eric.snowsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-09-11 22:54:11eric.snowsetkeywords: + patch
stage: patch review
pull_requests: + pull_request3500
2017-09-11 21:31:38eric.snowsetmessages: + msg301906
2017-09-11 21:21:13eric.snowsetmessages: + msg301904
2017-09-11 21:21:02eric.snowsetassignee: eric.snow
2017-09-11 20:48:58eric.snowsetnosy: + eric.snow
2017-09-11 17:16:19vstinnersetmessages: + msg301891
2017-09-11 17:14:04vstinnersetnosy: + ncoghlan, eric.smith
messages: + msg301889
2017-09-11 16:36:20skrahsetnosy: + skrah
messages: + msg301886
2017-09-11 16:30:05vstinnersetmessages: + msg301883
2017-09-11 16:29:21vstinnercreate