Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

__import__ with empty folder after importlib.invalidate_caches causes reference leak #80965

Closed
tirkarthi opened this issue May 3, 2019 · 5 comments
Closed

__import__ with empty folder after importlib.invalidate_caches causes reference leak #80965

tirkarthi opened this issue May 3, 2019 · 5 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@tirkarthi
Copy link
Member

BPO 36784
Nosy @brettcannon, @ncoghlan, @ericsnowcurrently, @JulienPalard, @tirkarthi, @skoslowski

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 2019-05-06.20:17:36.806>
created_at = <Date 2019-05-03.14:02:55.460>
labels = ['3.7', '3.8', 'type-bug', 'library', 'invalid']
title = '__import__ with empty folder after importlib.invalidate_caches causes reference leak'
updated_at = <Date 2019-05-06.20:21:03.708>
user = 'https://github.com/tirkarthi'

bugs.python.org fields:

activity = <Date 2019-05-06.20:21:03.708>
actor = 'xtreak'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-06.20:17:36.806>
closer = 'mdk'
components = ['Library (Lib)']
creation = <Date 2019-05-03.14:02:55.460>
creator = 'xtreak'
dependencies = []
files = []
hgrepos = []
issue_num = 36784
keywords = []
message_count = 5.0
messages = ['341338', '341375', '341638', '341639', '341640']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'ncoghlan', 'eric.snow', 'mdk', 'xtreak', 'skoslowski']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36784'
versions = ['Python 3.7', 'Python 3.8']
@tirkarthi
Copy link
Member Author

I originally hit upon this with bpo-36777 where I have used support.script_helper.make_script which calls importlib.invalidate_caches and then trying to use __import__ on an empty folder causes reference leak. I tried 3.8 to 3.5 and it exists on each version. A sample script as below. I tried using try finally instead of DirsOnSysPath in doubt and it still causes leak. I couldn't find any issues on search and let me know if I am using something in an incorrect manner.

import importlib
import unittest
import os, sys
import os.path
from test import support

def test_importlib_cache():

    with support.temp_dir() as path:
        dirname, basename = os.path.split(path)
        os.mkdir(os.path.join(path, 'test2'))
        importlib.invalidate_caches()

        with support.DirsOnSysPath(dirname):
            __import__("{basename}.test2".format(basename=basename))


class Tests(unittest.TestCase):

    def test_bug(self):
        for _ in range(10):
            test_importlib_cache()

➜ cpython git:(master) ✗ ./python.exe -m test -R 3:3 test_import_bug
Run tests sequentially
0:00:00 load avg: 1.56 [1/1] test_import_bug
beginning 6 repetitions
123456
......
test_import_bug leaked [980, 980, 980] references, sum=2940
test_import_bug leaked [370, 370, 370] memory blocks, sum=1110
test_import_bug failed

== Tests result: FAILURE ==

1 test failed:
test_import_bug

Total duration: 1 sec 529 ms
Tests result: FAILURE

I also tried __import__('test1.test2') instead of __import__("{basename}.test2".format(basename=basename)) and the program doesn't cause reference leak. Moving importlib.invalidate_caches() above support.temp_dir() also causes leak so I guess it's not something to do with temporary directories that are cleaned up after tests.

➜ cpython git:(master) ✗ mkdir -p test1/test2
➜ cpython git:(master) ✗ ./python.exe -m test -R 3:3 test_import_bug
Run tests sequentially
0:00:00 load avg: 1.97 [1/1] test_import_bug
beginning 6 repetitions
123456
......
test_import_bug passed

== Tests result: SUCCESS ==

1 test OK.

Total duration: 557 ms
Tests result: SUCCESS

@tirkarthi tirkarthi added 3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels May 3, 2019
@tirkarthi
Copy link
Member Author

Interesting, I used tracemalloc to see if it helps and it gave me a line in Lib/tempfile.py . Supplying path to support.temp_dir(path="/tmp/") causes random directory code was not to be hit and there was no memory leak. So I removed the _Random() initialization in _RandomNameSequence in Lib/tempfile.py and instead of self._rng.choice I used random.choice and still had the leak.

I replaced the code to generate random letters letters = [choose(c) for dummy in range(8)] where choose is random.choice with c = "a" and the memory leak stopped. I tried below combinations at line [0] and ran test to see if the memory leaks. I also tried -R 10:10 just to make sure my limits are higher enough. Using random.shuffle on a set of characters also causes leak. I am not sure why a combination of importlib.invalidate_caches, support.temp_dir using tempfile and __import__ causes these leaks or perhaps I am debugging or using huntrleaks in an incorrect manner.

# No leak

letters = [choose("a") for dummy in range(8)]
letters = ["a" for dummy in range(8)]
letters = [choose(self.characters[0]) for dummy in range(8)]

# Memory leak

letters = [choose("ab") for dummy in range(8)]
letters = [choose(self.characters[:]) for dummy in range(8)]
letters = [choose(list(self.characters)) for dummy in range(8)]

# Below also leaks

characters = list("abcde")  # list("abcd") doesn't leak
self.rng.shuffle(characters)
letters = characters[:8]


from unittest import TestCase
import tracemalloc
import sys
import os
from test import support

def test_importlib_cache_tempdir():

    import importlib
    importlib.invalidate_caches()

    with support.temp_dir() as path:   # with support.temp_dir(path="/tmp") as path: (no leak)
        dirname = os.path.dirname(path)
        basename = os.path.basename(path)
        os.mkdir(os.path.join(path, 'test2'))

        with support.DirsOnSysPath(dirname):
            __import__(f"{basename}.test2".format(basename=basename))


class Tests(TestCase):

    def test_bug(self):
        tracemalloc.start()

        for _ in range(10):
            test_importlib_cache_tempdir()

        snapshot = tracemalloc.take_snapshot()
        top_stats = snapshot.statistics('traceback')

        print("[ Top 10 ]")
        for stat in top_stats[:10]:
            for line in stat.traceback.format():
                print(line)
$ ./python.exe -m test -R 3:3 test_import_bug_tempdir
Run tests sequentially
0:00:00 load avg: 2.55 [1/1] test_import_bug_tempdir
beginning 6 repetitions
123456
[ Top 10 ]
  File "<frozen importlib._bootstrap_external>", line 1486
  File "<frozen importlib._bootstrap_external>", line 1461
  File "<frozen importlib._bootstrap_external>", line 1469
  File "<frozen importlib._bootstrap>", line 683
  File "<frozen importlib._bootstrap>", line 509
  File "<frozen importlib._bootstrap_external>", line 1378
  File "<frozen importlib._bootstrap>", line 344
  File "<frozen importlib._bootstrap>", line 36
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/tempfile.py", line 136
    self._rng = _Random()
  File "<frozen importlib._bootstrap_external>", line 1342
.[ Top 10 ]
  File "<frozen importlib._bootstrap_external>", line 1486
  File "<frozen importlib._bootstrap_external>", line 1461
  File "<frozen importlib._bootstrap_external>", line 1469
  File "<frozen importlib._bootstrap>", line 683
  File "<frozen importlib._bootstrap>", line 509
  File "<frozen importlib._bootstrap>", line 344
  File "<frozen importlib._bootstrap>", line 36
  File "<frozen importlib._bootstrap_external>", line 64
  File "<frozen importlib._bootstrap_external>", line 1342
  File "<frozen importlib._bootstrap_external>", line 1378
.[ Top 10 ]
  File "<frozen importlib._bootstrap_external>", line 1486
  File "<frozen importlib._bootstrap_external>", line 1461
  File "<frozen importlib._bootstrap_external>", line 1469
  File "<frozen importlib._bootstrap>", line 683
  File "<frozen importlib._bootstrap>", line 509
  File "<frozen importlib._bootstrap>", line 344
  File "<frozen importlib._bootstrap>", line 36
  File "<frozen importlib._bootstrap_external>", line 64
  File "<frozen importlib._bootstrap_external>", line 1342
  File "<frozen importlib._bootstrap_external>", line 1132
.[ Top 10 ]
  File "<frozen importlib._bootstrap_external>", line 1486
  File "<frozen importlib._bootstrap_external>", line 1461
  File "<frozen importlib._bootstrap_external>", line 1469
  File "<frozen importlib._bootstrap>", line 509
  File "<frozen importlib._bootstrap>", line 683
  File "<frozen importlib._bootstrap>", line 344
  File "<frozen importlib._bootstrap>", line 36
  File "<frozen importlib._bootstrap_external>", line 64
  File "<frozen importlib._bootstrap_external>", line 1342
  File "<frozen importlib._bootstrap_external>", line 1132
.[ Top 10 ]
  File "<frozen importlib._bootstrap_external>", line 1486
  File "<frozen importlib._bootstrap_external>", line 1461
  File "<frozen importlib._bootstrap_external>", line 1469
  File "<frozen importlib._bootstrap>", line 509
  File "<frozen importlib._bootstrap>", line 683
  File "<frozen importlib._bootstrap>", line 344
  File "<frozen importlib._bootstrap>", line 36
  File "<frozen importlib._bootstrap_external>", line 64
  File "<frozen importlib._bootstrap_external>", line 1342
  File "<frozen importlib._bootstrap_external>", line 1132
.[ Top 10 ]
  File "<frozen importlib._bootstrap_external>", line 1486
  File "<frozen importlib._bootstrap_external>", line 1461
  File "<frozen importlib._bootstrap_external>", line 1469
  File "<frozen importlib._bootstrap>", line 509
  File "<frozen importlib._bootstrap>", line 683
  File "<frozen importlib._bootstrap>", line 344
  File "<frozen importlib._bootstrap>", line 36
  File "<frozen importlib._bootstrap_external>", line 64
  File "<frozen importlib._bootstrap_external>", line 1342
  File "<frozen importlib._bootstrap_external>", line 1132
.
test_import_bug_tempdir leaked [980, 980, 980] references, sum=2940
test_import_bug_tempdir leaked [370, 370, 370] memory blocks, sum=1110
test_import_bug_tempdir failed

== Tests result: FAILURE ==

1 test failed:
test_import_bug_tempdir

Total duration: 3 sec 254 ms
Tests result: FAILURE

@skoslowski
Copy link
Mannequin

skoslowski mannequin commented May 6, 2019

So, I dug into this here at the PyCon19 sprints and as far as I can see there is no actual leak.

What you are seeing in your code example is from the state, that is kept between successive run of your import. All the cases you reported as not leaking generate a fixed tempdir. However, if the tempdir is random (or at least differs between runs) two new modules are added to sys.modules and one entry is added to the path_importer_cache for each run. These are not cleared by invalidate_caches().

If you append the following lines to test_importlib_cache_tempdir() these objects (and the caches in them) get cleared and your test passes.

    sys.modules.pop(basename + ".test2")
    sys.modules.pop(basename)
    sys.path_importer_cache.pop(path)

This can also be confirmed using sys.gettotalrefcount().

@JulienPalard
Copy link
Member

Thanks Sebastian for looking at it \o/

@tirkarthi
Copy link
Member Author

Thanks for the details.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@JulienPalard @tirkarthi