Adding tests to test_import.SubinterpImportTests causes refleak on FreeBSD

[encukou]

Adding a few tests to test_import.SubinterpImportTests causes a refleak to be detected on FreeBSD:

test_import leaked [498, 498, 498] references, sum=1494
test_import leaked [326, 326, 326] memory blocks, sum=978

As intriguing as this is, at this point I don't think I should spend more time investigating it. Perhaps someone has a quick idea on what to try :)

cc @vstinner as an expert on FreeBSD and refleak hunting.

See PR and buildbot run. The patch I used is:

diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index b71a36ec2f7aab..febfb96a53e46c 100644
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -2467,6 +2467,30 @@ def test_multi_init_extension_per_interpreter_gil_compat(self):
                         self.check_compatible_here(
                             modname, filename, strict=False, isolated=False)
 
+    def test_extra_A(self):
+        return
+
+    def test_extra_B(self):
+        return
+
+    def test_extra_C(self):
+        return
+
+    def test_extra_D(self):
+        return
+
+    def test_extra_E(self):
+        return
+
+    def test_extra_F(self):
+        return
+
+    def test_extra_G(self):
+        return
+
+    def test_extra_H(self):
+        return
+
     @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi")
     def test_python_compat(self):
         module = 'threading'

Linked PRs

• gh-140657: (bug trigger) Add extra no-op tests to test_import.SubinterpImportTests #140655
• gh-140657: Run test_import single phase init tests in a subprocess #140708
• gh-140657: Don't rerun test_import single phase init test #140712
• [3.14] gh-140657: Don't rerun test_import single phase init test (GH-140712) #140713
• [3.13] gh-140657: Don't rerun test_import single phase init test (GH-140712) #140714

[vstinner]

This test leaks on FreeBSD:

import _testinternalcapi
import importlib.util
import os
import sys
import textwrap
import unittest
from importlib.machinery import ExtensionFileLoader
from test.support import run_in_subinterp, gc_collect


class SinglephaseInitTests(unittest.TestCase):
    def test_basic_multiple_interpreters_main_no_reset(self):
        name = '_testsinglephase'
        spec = importlib.util.find_spec(name)
        origin = spec.origin

        r, w = os.pipe()
        self.addCleanup(os.close, r)
        self.addCleanup(os.close, w)

        script = textwrap.dedent('''
            import sys
            name = '_testsinglephase'
            assert name not in sys.modules

            import _testsinglephase
            import json
        ''')

        ret = run_in_subinterp(script)
        if ret != 0:
            raise AssertionError(f'{ret} != 0')

        if name in sys.modules:
            sys.modules[name]._clear_globals()
            del sys.modules[name]
        _testinternalcapi.clear_extension(name, origin)

        gc_collect()

Output:

$ ./python -m test test_leak -R 3:3
...
test_leak leaked [27, 11, 27] references, sum=65
test_leak leaked [11, 11, 11] memory blocks, sum=33

[vstinner]

The fix for test_leak.py is to call _testinternalcapi.clear_extension() in the sub-interpreter:

import _testinternalcapi
import importlib.util
import os
import sys
import textwrap
import unittest
from importlib.machinery import ExtensionFileLoader
from test.support import run_in_subinterp, gc_collect


class SinglephaseInitTests(unittest.TestCase):
    def test_basic_multiple_interpreters_main_no_reset(self):
        name = '_testsinglephase'
        spec = importlib.util.find_spec(name)
        origin = spec.origin

        script = textwrap.dedent('''
            name = '_testsinglephase'

            import importlib.util
            import sys

            spec = importlib.util.find_spec(name)
            origin = spec.origin
            assert name not in sys.modules

            import _testsinglephase
            import json
            import _testinternalcapi

            _testinternalcapi.clear_extension(name, origin)
        ''')

        ret = run_in_subinterp(script)
        if ret != 0:
            raise AssertionError(f'{ret} != 0')

[encukou]

But, add_subinterpreter adds a cleanup function that runs clear_extension in the sub-interpreter.

[vstinner]

If I add a GC collection with the patch below, the test seems to leak memory:

vstinner@freebsd$ ./python -m test test_import -m test_basic_multiple_interpreters_main_no_reset -R 3:3 
(...)
test_import leaked [44, 0, 44] references, sum=88 (this is fine)
test_import leaked [25, 1, 25] memory blocks, sum=51
0:00:02 load avg: 0.99 [1/1/1] test_import failed (reference leak)

I can reproduce this issue on Linux.

Patch:

diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index b71a36ec2f7..1a095bd1120 100644
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -1,5 +1,6 @@
 import builtins
 import errno
+import gc
 import glob
 import json
 import importlib.util
@@ -3228,6 +3229,8 @@ def test_basic_multiple_interpreters_main_no_reset(self):
         #  * m_copy was copied from interp2 (was from interp1)
         #  * module's global state was updated, not reset
 
+        gc.collect()
+
     @no_rerun(reason="rerun not possible; module state is never cleared (see gh-102251)")
     @requires_subinterpreters
     def test_basic_multiple_interpreters_deleted_no_reset(self):

[vstinner]

But, add_subinterpreter adds a cleanup function that runs clear_extension in the sub-interpreter.

In test_basic_multiple_interpreters_main_no_reset():

        # Use an interpreter that gets destroyed right away.
        loaded = self.import_in_subinterp()

It calls import_in_subinterp() with postcleanup=False.

If I set postcleanup=True, the test fails with:

Traceback (most recent call last):
  File "/usr/home/vstinner/python/main/Lib/test/test_import/__init__.py", line 3211, in test_basic_multiple_interpreters_main_no_reset
    self.check_copied(loaded, main_loaded)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
  File "/usr/home/vstinner/python/main/Lib/test/test_import/__init__.py", line 2957, in check_copied
    self.assertEqual(snap.state_initialized,
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
                     base.snapshot.state_initialized)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 75297.937840862 != 75297.892977397

[encukou]

Well, marking this @no_rerun (like the test_basic_multiple_interpreters_deleted_no_reset) would hide the issue.