[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:

> Code extremely simplified: (...)

Relationship between origin unmodified code and the extremely simplified code:

* Lib/test/test_importlib/__init__.py: load_tests() is called at each "test -R 
3:3" iteration
* Lib/test/test_importlib/test_windows.py: at each iteartion:

  * machinery = 
test.test_importlib.util.import_importlib('importlib.machinery') is called: it 
reloads the whole importlib package, with _frozen_importlib and 
_frozen_importlib_external blocked (force to reload pure Python 
importlib._bootstrap and importlib._bootstrap_external modules)
  * support.import_module('winreg', required_on=['win']) is called with the 
fresh new importlib package: in short, it tries to import winreg which fails 
with ModuleNotFoundError

* Lib/importlib/__init__.py: each time the module is reloaded

  * importlib._bootstrap._setup() is called: it copies '_thread', '_warnings' 
and '_weakref' from sys.modules into importlib._bootstrap namespace (module 
globals) using setattr(self_module, ...)
  * importlib._bootstrap_external._setup() is called: it calls 
_bootstrap._builtin_from_name('_weakref')


When importlib._bootstrap_external._setup() calls 
_bootstrap._builtin_from_name('_weakref'), _init_module_attrs() copies the 
"spec" into module.__spec__. But the spec contains a reference to 
importlib._bootstrap namespace.


Before _weakref is converted to multiphase initialization:

$ ./python
>>> import _weakref
>>> id(_weakref)
140119879580176
>>> id(_weakref.__spec__.__init__.__globals__['_weakref'])
140119879580176

=> same module


After the change:

$ ./python
>>> import _weakref
>>> id(_weakref)
140312826159952
>>> id(_weakref.__spec__.__init__.__globals__['_weakref'])
140312826366288

=> two different objects


The problem is that module.__spec__ pulls the whole importlib package which 
contains tons of objects.

--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:

Code extremely simplified:
---
import unittest
import sys
import _imp

class ModuleSpec:
pass

class Tests(unittest.TestCase):
def test_import_fresh(self):
spec = ModuleSpec()
spec.sys_weakref = sys.modules["_weakref"]
spec.name = "_weakref"
module = _imp.create_builtin(spec)
module.__spec__ = spec
sys.modules["_weakref"] = module
---

I still get a leak:
---
test_leak leaked [34, 34, 34] references, sum=102
test_leak leaked [11, 11, 11] memory blocks, sum=33
---

I understand that sys.modules["_weakref"] is overriden by the test with a new 
module, and the new module holds a reference somehow to the previous _weakref 
module. The old and the new modules remain alive.

--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:

> File 1:

Oops, you should read:

File 1: Lib/test/test_leak.py

--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:

With the latest Lib/importlib3.py, there are less leaked references, but there 
are still leaked references:

test_leak leaked [139, 139, 139] references, sum=417
test_leak leaked [42, 42, 42] memory blocks, sum=126

--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:

I can reproduce the leak with command:

./python -m test -R 3:3 test_leak

File 1:
---
import unittest
import sys
from importlib import _bootstrap as BOOTSTRAP


def _save_and_remove_module(name, orig_modules):
"""Helper function to save and remove a module from sys.modules

Raise ImportError if the module can't be imported.
"""
# try to import the module and raise an error if it can't be imported
if name not in sys.modules:
__import__(name)
del sys.modules[name]
for modname in list(sys.modules):
if modname == name or modname.startswith(name + '.'):
orig_modules[modname] = sys.modules[modname]
del sys.modules[modname]

def _save_and_block_module(name, orig_modules):
"""Helper function to save and block a module in sys.modules

Return True if the module was in sys.modules, False otherwise.
"""
saved = True
try:
orig_modules[name] = sys.modules[name]
except KeyError:
saved = False
sys.modules[name] = None
return saved



def import_fresh_module():
name = 'importlib3'
fresh = ('importlib',)
blocked = ('_frozen_importlib', '_frozen_importlib_external')
orig_modules = {}
names_to_remove = []
_save_and_remove_module(name, orig_modules)
try:
for fresh_name in fresh:
_save_and_remove_module(fresh_name, orig_modules)
for blocked_name in blocked:
if not _save_and_block_module(blocked_name, orig_modules):
names_to_remove.append(blocked_name)

with BOOTSTRAP._ModuleLockManager(name):
spec = BOOTSTRAP._find_spec(name, None)
module = BOOTSTRAP.module_from_spec(spec)
sys.modules[spec.name] = module
spec.loader.exec_module(module)
del sys.modules[spec.name]


finally:
for orig_name, module in orig_modules.items():
sys.modules[orig_name] = module
for name_to_remove in names_to_remove:
del sys.modules[name_to_remove]

class Tests(unittest.TestCase):
def test_import_fresh(self):
import_fresh_module()
---

File 2: Lib/importlib3.py
---
import _imp
import sys

try:
import _frozen_importlib as _bootstrap
case = 1
except ImportError:
case = 2

if case == 2:
_bootstrap = type(sys)("_bootstrap")
_bootstrap._weakref = sys.modules['_weakref']

class ModuleSpec:
@staticmethod
def method():
pass

spec = ModuleSpec()
spec.name = "_weakref"

module = _imp.create_builtin(spec)
module.__spec__ = spec

sys.modules["_weakref"] = module
---

--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[hai shi]

Change by hai shi :


--
nosy: +shihai1991

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:

Before commit 8334f30a74abcf7e469b901afc307887aa85a888, at the first 
_imp.create_builtin() calls, it calls _PyImport_FixupExtensionObject() which 
stores the created module in an internal "extensions" dictionary. 
PyInit__weakref() returns a module object. Next calls to 
_imp.create_builtin("_weakref") simply returned the cached _weakref module.

At commit 8334f30a74abcf7e469b901afc307887aa85a888, _imp.create_builtin() 
doesn't store it in the internal "extensions" dictionary, but calls 
PyModule_FromDefAndSpec() instead. PyInit__weakref() returns a module 
definition (PyModuleDef_Type: "moduledef"). Each 
_imp.create_builtin("_weakref") creates a new module.

--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:

I can reproduce the leak with the following test, stored as 
Lib/test/test_leak.py:
---
from test import support
import unittest

class Tests(unittest.TestCase):
def test_import_fresh(self):
support.import_fresh_module('importlib.machinery',
fresh=('importlib',),
blocked=('_frozen_importlib', '_frozen_importlib_external'))
---

Extract of "./python -m test -R 3:3 test_leak" output:
---
test_leak leaked [6303, 6299, 6303] references, sum=18905
test_leak leaked [2022, 2020, 2022] memory blocks, sum=6064
---

--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 188078c39dec24aa5b3f2073bdc9a68ebaae42de by Victor Stinner in 
branch 'master':
Revert "bpo-1635741: Port _weakref extension module to multiphase 
initialization (PEP 489) (GH-19084)" (#19128)
https://github.com/python/cpython/commit/188078c39dec24aa5b3f2073bdc9a68ebaae42de


--

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch
pull_requests: +18490
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/19128

___
Python tracker 

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

[issue40050] test_importlib leaked [6303, 6299, 6303] references

[STINNER Victor]

New submission from STINNER Victor :

https://buildbot.python.org/all/#/builders/206/builds/119

test_importlib leaked [6303, 6299, 6303] references, sum=18905
test_importlib leaked [2022, 2020, 2022] memory blocks, sum=6064

Issue reported at: https://bugs.python.org/issue1635741#msg364845

It seems like the regression was introduced by the following change:

commit 8334f30a74abcf7e469b901afc307887aa85a888 (HEAD)
Author: Hai Shi 
Date:   Fri Mar 20 16:16:45 2020 +0800

bpo-1635741: Port _weakref extension module to multiphase initialization 
(PEP 489) (GH-19084)

--
components: Tests
messages: 364905
nosy: vstinner
priority: normal
severity: normal
status: open
title: test_importlib leaked [6303, 6299, 6303] references
versions: Python 3.9

___
Python tracker 

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