classification
Title: _signal module leak: test_interpreters leaked [1424, 1422, 1424] references
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: koubaa, shihai1991, vstinner
Priority: normal Keywords: patch

Created on 2020-09-04 09:53 by vstinner, last changed 2020-11-17 22:29 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 22087 merged vstinner, 2020-09-04 11:50
PR 23318 closed vstinner, 2020-11-16 17:51
PR 23342 merged vstinner, 2020-11-17 14:38
PR 23344 merged vstinner, 2020-11-17 15:27
PR 23347 merged vstinner, 2020-11-17 17:26
PR 23349 merged vstinner, 2020-11-17 18:02
PR 23355 merged vstinner, 2020-11-17 22:07
Messages (14)
msg376345 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-04 09:53
AMD64 Fedora Rawhide Refleaks 3.x:
https://buildbot.python.org/all/#/builders/565/builds/11

test_interpreters leaked [1424, 1422, 1424] references, sum=4270

According to git bisect, the leak was introduced by:

commit 71d1bd9569c8a497e279f2fea6fe47cd70a87ea3
Author: Mohamed Koubaa <koubaa.m@gmail.com>
Date:   Thu Sep 3 03:21:06 2020 -0500

    bpo-1635741: Port _signal module to multi-phase init (PEP 489) (GH-22049)

 .../2020-09-01-17-07-20.bpo-1635741.7wSuCc.rst     |   1 +
 Modules/signalmodule.c                             | 168 +++++++++++----------
 2 files changed, 87 insertions(+), 82 deletions(-)


Example of leak:

$ ./python -m test -R 3:3 test_interpreters -m test.test_interpreters.TestInterpreterClose.test_from_current
0:00:00 load avg: 0.72 Run tests sequentially
0:00:00 load avg: 0.72 [1/1] test_interpreters
beginning 6 repetitions
123456
......
test_interpreters leaked [237, 237, 237] references, sum=711
test_interpreters leaked [18, 18, 18] memory blocks, sum=54
test_interpreters failed

== Tests result: FAILURE ==

1 test failed:
    test_interpreters

Total duration: 1.1 sec
Tests result: FAILURE
msg376348 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-04 10:03
The signal module is really special. In Python, only the main thread of the main interpreter is supposed to be allowed to run Python signal handlers (but the C signal handler can be executed by any thread).

The exec function of the _signal module runs actions each time a new instance of the _signal module is created, whereas some actions must only be done exactly once:

* modify Handlers[i].tripped
* modify Handlers[i].func
* Set SIGINT signal handler
* Initialize sigint_event event (Windows only): see bpo-41686

For example, calling signal.signal() in a subinterpreter raises ValueError("signal only works in main thread of the main interpreter").
msg376349 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-04 10:08
About the leak, the following three variables are also initialized multiple times by signal_exec():

static PyObject *DefaultHandler;
static PyObject *IgnoreHandler;
static PyObject *IntHandler;
msg376359 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-04 11:50
Another problem: PyOS_FiniInterrupts() is a public function of the C API which access global variables like IntHandler, DefaultHandler and IgnoreHandler.
msg376364 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-04 12:51
New changeset 4b8032e5a4994a7902076efa72fca1e2c85d8b7f by Victor Stinner in branch 'master':
bpo-41713: _signal doesn't use multi-phase init (GH-22087)
https://github.com/python/cpython/commit/4b8032e5a4994a7902076efa72fca1e2c85d8b7f
msg376469 - (view) Author: mohamed koubaa (koubaa) * Date: 2020-09-06 21:41
Sounds like there needs to be some python-wide global state that perhaps the signal module wraps rather than owns.  I could try to prototype it if you agree
msg376498 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-07 14:30
See also:

* bpo-40600: "Add an option to disallow creating more than one instance of a module".
* bpo-40288: [subinterpreters] atexit module should not be loaded more than once per interpreter
msg381246 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 15:22
New changeset 296a796951032f678d063008f588ccc6958d0df1 by Victor Stinner in branch 'master':
bpo-41713: Remove PyOS_InitInterrupts() function (GH-23342)
https://github.com/python/cpython/commit/296a796951032f678d063008f588ccc6958d0df1
msg381258 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 17:15
New changeset 0ae323b87f1bed64a7fa70f5a41a5800aca032cc by Victor Stinner in branch 'master':
bpo-41686: Always create the SIGINT event on Windows (GH-23344)
https://github.com/python/cpython/commit/0ae323b87f1bed64a7fa70f5a41a5800aca032cc
msg381267 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 17:58
New changeset 05a5d697f4f097f37c5c1e2ed0e2338a33c3fb6a by Victor Stinner in branch '3.9':
bpo-41686: Always create the SIGINT event on Windows (GH-23344) (GH-23347)
https://github.com/python/cpython/commit/05a5d697f4f097f37c5c1e2ed0e2338a33c3fb6a
msg381287 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 21:23
New changeset a702bd4b921167e73f8fc987aa64ada571fdc3f8 by Victor Stinner in branch '3.8':
bpo-41686: Always create the SIGINT event on Windows (GH-23344) (GH-23347) (GH-23349)
https://github.com/python/cpython/commit/a702bd4b921167e73f8fc987aa64ada571fdc3f8
msg381293 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 21:56
New changeset 29aa624047f893b3b3194f00252b2156bbbf4f9b by Victor Stinner in branch 'master':
bpo-41686: Move _Py_RestoreSignals() to signalmodule.c (GH-23353)
https://github.com/python/cpython/commit/29aa624047f893b3b3194f00252b2156bbbf4f9b
msg381298 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 22:28
New changeset 7f9b25a21ab95f8cf8d663396993766307be475c by Victor Stinner in branch 'master':
bpo-41713: Port _signal module to multi-phase init (GH-23355)
https://github.com/python/cpython/commit/7f9b25a21ab95f8cf8d663396993766307be475c
msg381299 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 22:29
Done! _signal uses again the multi-phase init API.
History
Date User Action Args
2020-11-19 13:59:44vstinnerlinkissue15707 superseder
2020-11-17 22:29:15vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg381299

stage: patch review -> resolved
2020-11-17 22:28:49vstinnersetmessages: + msg381298
2020-11-17 22:07:37vstinnersetpull_requests: + pull_request22248
2020-11-17 21:56:32vstinnersetmessages: + msg381293
2020-11-17 21:23:25vstinnersetmessages: + msg381287
2020-11-17 18:02:29vstinnersetpull_requests: + pull_request22243
2020-11-17 17:58:18vstinnersetmessages: + msg381267
2020-11-17 17:26:41vstinnersetpull_requests: + pull_request22240
2020-11-17 17:15:29vstinnersetmessages: + msg381258
2020-11-17 15:27:10vstinnersetpull_requests: + pull_request22236
2020-11-17 15:22:31vstinnersetmessages: + msg381246
2020-11-17 14:38:36vstinnersetpull_requests: + pull_request22233
2020-11-16 17:51:41vstinnersetpull_requests: + pull_request22209
2020-09-07 14:30:08vstinnersetmessages: + msg376498
2020-09-06 21:41:40koubaasetnosy: + koubaa
messages: + msg376469
2020-09-04 14:05:37shihai1991setnosy: + shihai1991
2020-09-04 12:51:19vstinnersetmessages: + msg376364
2020-09-04 11:50:24vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request21173
2020-09-04 11:50:02vstinnersetmessages: + msg376359
2020-09-04 10:08:55vstinnersetmessages: + msg376349
2020-09-04 10:03:39vstinnersetmessages: + msg376348
2020-09-04 09:55:36vstinnersettitle: test_interpreters leaked [1424, 1422, 1424] references -> _signal module leak: test_interpreters leaked [1424, 1422, 1424] references
2020-09-04 09:53:41vstinnercreate