_signal module leak: test_interpreters leaked [1424, 1422, 1424] references

[vstinner]

BPO	41713
Nosy	@vstinner, @encukou, @shihai1991, @koubaa
PRs	bpo-41713: _signal doesn't use multi-phase init #22087 [WIP] bpo-41713: Move _signal variables into a state structure #23318 bpo-41713: Remove PyOS_InitInterrupts() function #23342 bpo-41686: Always create the SIGINT event on Windows #23344 [3.9] bpo-41686: Always create the SIGINT event on Windows (GH-23344) #23347 [3.8] bpo-41686: Always create the SIGINT event on Windows (GH-23344) (GH-23347) #23349 bpo-41713: Port _signal module to multi-phase init #23355 bpo-41713: Remove PyOS_InitInterrupts() from python3dll.c #24257

^{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:

assignee = None
closed_at = <Date 2020-11-17.22:29:15.934>
created_at = <Date 2020-09-04.09:53:41.602>
labels = ['interpreter-core', '3.10']
title = '_signal module leak: test_interpreters leaked [1424, 1422, 1424] references'
updated_at = <Date 2021-04-27.23:34:44.792>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2021-04-27.23:34:44.792>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-17.22:29:15.934>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2020-09-04.09:53:41.602>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 41713
keywords = ['patch']
message_count = 18.0
messages = ['376345', '376348', '376349', '376359', '376364', '376469', '376498', '381246', '381258', '381267', '381287', '381293', '381298', '381299', '385268', '385275', '385280', '392153']
nosy_count = 4.0
nosy_names = ['vstinner', 'petr.viktorin', 'shihai1991', 'koubaa']
pr_nums = ['22087', '23318', '23342', '23344', '23347', '23349', '23355', '24257']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue41713'
versions = ['Python 3.10']

[vstinner]

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 71d1bd9
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

[vstinner]

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").

[vstinner]

About the leak, the following three variables are also initialized multiple times by signal_exec():

static PyObject *DefaultHandler;
static PyObject *IgnoreHandler;
static PyObject *IntHandler;

[vstinner]

Another problem: PyOS_FiniInterrupts() is a public function of the C API which access global variables like IntHandler, DefaultHandler and IgnoreHandler.

[vstinner]

New changeset 4b8032e by Victor Stinner in branch 'master':
bpo-41713: _signal doesn't use multi-phase init (GH-22087)
4b8032e

[koubaa]

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

[vstinner]

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

[vstinner]

New changeset 296a796 by Victor Stinner in branch 'master':
bpo-41713: Remove PyOS_InitInterrupts() function (GH-23342)
296a796

[vstinner]

New changeset 0ae323b by Victor Stinner in branch 'master':
bpo-41686: Always create the SIGINT event on Windows (GH-23344)
0ae323b

[vstinner]

New changeset 05a5d69 by Victor Stinner in branch '3.9':
bpo-41686: Always create the SIGINT event on Windows (GH-23344) (GH-23347)
05a5d69

[vstinner]

New changeset a702bd4 by Victor Stinner in branch '3.8':
bpo-41686: Always create the SIGINT event on Windows (GH-23344) (GH-23347) (GH-23349)
a702bd4

[vstinner]

New changeset 29aa624 by Victor Stinner in branch 'master':
bpo-41686: Move _Py_RestoreSignals() to signalmodule.c (GH-23353)
29aa624