deepmodeling / deepmodeling/deepmd-kit
[Code scan] Close merge_lmdb source environments through the LMDB cache
- Dominant language
- Python
- Stars
- 2k
- Forks
- 649
- Avg merge
- 6d 18h
- Merged PRs (30d)
- 15
Description
Found during a Codex global scan of `deepmodeling/deepmd-kit` at commit `73de44b1f94471b2e3bdb6b11f57b34d7bc791bb`.
## Problem
`merge_lmdb()` opens source environments through the shared LMDB cache but closes them directly, bypassing the cache refcount.
Evidence:
- `_open_lmdb()` stores environments in `_ENV_CACHE` and increments a refcount for reused paths: https://github.com/deepmodeling/deepmd-kit/blob/73de44b1f94471b2e3bdb6b11f57b34d7bc791bb/deepmd/dpmodel/utils/lmdb_data.py#L51-L74
- `_close_lmdb()` is the matching refcount-aware close path: https://github.com/deepmodeling/deepmd-kit/blob/73de44b1f94471b2e3bdb6b11f57b34d7bc791bb/deepmd/dpmodel/utils/lmdb_data.py#L77-L91
- `merge_lmdb()` calls `_open_lmdb(src_path)` for every source: https://github.com/deepmodeling/deepmd-kit/blob/73de44b1f94471b2e3bdb6b11f57b34d7bc791bb/deepmd/dpmodel/utils/lmdb_data.py#L1767-L1769
- It later calls `src_env.close()` directly: https://github.com/deepmodeling/deepmd-kit/blob/73de44b1f94471b2e3bdb6b11f57b34d7bc791bb/deepmd/dpmodel/utils/lmdb_data.py#L1816-L1822
## Impact
The cache can retain a closed environment, and an active `LmdbDataReader` sharing the same cached environment can be invalidated by a merge. Later readers may reuse the closed handle instead of opening a fresh environment.
## Suggested Fix
Replace the direct close with `_close_lmdb(src_path)` in a `finally` block. Add a regression test that opens a reader, calls `merge_lmdb()` on the same path, and then verifies the existing reader and a new reader can still read frames.
Contributor guide
Assessment
This issue has not been assessed yet.