bpo-1635741: Port _gdbm module to multiphase initialization by corona10 · Pull Request #20920 · python/cpython

@corona10

@corona10

@corona10

I' ve checked the memory leak with these scripts and no leaks found

./python.exe -m test test_dbm_gnu -R 3:3
def test_gdbm(self):
    code = textwrap.dedent(r"""
        import glob

        import test.support
        dbm = test.support.import_module('_gdbm')
        # or dbm = test.support.import_module('dbm')
        _fname = test.support.TESTFN

        def delete_files():
            for f in glob.glob(_fname + "*"):
                test.support.unlink(f)

        f = dbm.open(_fname, 'n')
        f[b'g'] = b"indented"
        f.close()
        delete_files()
    """)
    ret = test.support.run_in_subinterp(code)
    self.assertEqual(ret, 0)

@corona10

Note that dbm = test.support.import_module('dbm') is leaked at the master branch :)

vstinner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Minor remarks:

  • I'm not sure why only a few methods uses the defining class, whereas some others use Py_TYPE(self).
  • nitpick: some "clinic input" uses an empty line betwen the function name and its parameters, sometimes there is none.

But I don't think that it's worth it to address these minor things. If you care, you may also update _dbmmodule.c. Maybe in a follow-up PRs.

@vstinner

Note that dbm = test.support.import_module('dbm') is leaked at the master branch :)

Maybe open an issue to track this bug.

@vstinner

@corona10

@vstinner

Maybe open an issue to track this bug

Oh I mean the subinterpreter test with dbm = test.support.import_module('dbm')is leaked
With this change, there is no leak.

@corona10

@corona10

@corona10

@vstinner

With this change, there is no leak.

Oh ok, that's great! Sorry, I misunderstood your comment.

@kylotan kylotan mannequin mentioned this pull request

Sep 19, 2022