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.

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

leak if we use the "try dict[key] / except KeyError" idiom with h5py attrs #2350

Open
bomangus opened this issue Dec 5, 2023 · 7 comments
Open

Comments

@bomangus
Copy link

bomangus commented Dec 5, 2023

h5py never releases data if we use the 'try dict[key] / except KeyError' idiom with h5py attrs.

On fc39: python3-3.12.0, hdf5-1.12.1, python3-h5py-3.10.0 (leaks)
On fc36: python3-3.10.11, hdf5-1.12.1, python3-h5py-3.6.0 (no leak)

Output from attached script on Fedora 39 (leaks)

See in particular the second dataset.py tracemalloc line with 10x the count and memory expected.

fc39$ h5-leak.py FOO.h5
Summary of the h5py configuration
---------------------------------

h5py    3.10.0
HDF5    1.12.1
Python  3.12.0 (main, Oct  2 2023, 00:00:00) [GCC 13.2.1 20230918 (Red Hat 13.2.1-3)]
sys.platform    linux
sys.maxsize     9223372036854775807
numpy   1.24.4
cython (built with) 3.0.2
numpy (built against) 1.24.4
HDF5 (built against) 1.12.1

GOOD x10
/usr/lib64/python3.12/site-packages/h5py/_hl/dataset.py:758: size=352 KiB, count=3, average=117 KiB
/usr/lib64/python3.12/site-packages/h5py/_hl/dataset.py:645: size=4624 B, count=1, average=4624 B
/usr/lib64/python3.12/site-packages/h5py/_hl/group.py:357: size=4440 B, count=37, average=120 B
LEAKY x10
/usr/lib64/python3.12/site-packages/h5py/_hl/dataset.py:758: size=3517 KiB, count=29, average=121 KiB
/usr/lib64/python3.12/site-packages/h5py/_hl/attrs.py:56: size=17.0 KiB, count=124, average=141 B
/usr/lib64/python3.12/site-packages/h5py/_hl/group.py:357: size=15.5 KiB, count=78, average=203 B

Output from attached script on Fedora 36 (no leak)

fc36$ h5-leak.py FOO.h5
Summary of the h5py configuration
---------------------------------

h5py    3.6.0
HDF5    1.12.1
Python  3.10.11 (main, Apr  5 2023, 00:00:00) [GCC 12.2.1 20221121 (Red Hat 12.2.1-4)]
sys.platform    linux
sys.maxsize     9223372036854775807
numpy   1.22.0
cython (built with) 0.29.26
numpy (built against) 1.22.0
HDF5 (built against) 1.12.1

GOOD x10
/usr/lib64/python3.10/site-packages/h5py/_hl/dataset.py:710: size=352 KiB, count=4, average=88.0 KiB
/usr/lib64/python3.10/site-packages/h5py/_hl/group.py:305: size=6776 B, count=39, average=174 B
/usr/lib64/python3.10/site-packages/h5py/_hl/attrs.py:66: size=5008 B, count=2, average=2504 B
LEAKY x10
/usr/lib64/python3.10/site-packages/h5py/_hl/dataset.py:710: size=352 KiB, count=4, average=88.0 KiB
/usr/lib64/python3.10/site-packages/h5py/_hl/group.py:305: size=6776 B, count=39, average=174 B
/usr/lib64/python3.10/site-packages/h5py/_hl/base.py:150: size=4632 B, count=1, average=4632 B

Script

# script to reproduce h5py try-attrs-KeyError bug
#
# USAGE: python3 h5-leak.py PATH
#
# h5py leaks if we use the 'try dict[key] / except KeyError' idiom with h5py attrs

import sys
import tracemalloc

import h5py

def odim_data_leaky(path):
    vals = []
    with h5py.File(path, "r") as hfile:
        data = hfile["dataset1/data1/data"][:]
        d_what = hfile["dataset1/data1/what"]
        for k in ATTR_KEYS:
            try:
                val = d_what.attrs[k]
                vals.append(val)
            except KeyError:
                # NB: if we get here, we may abandon a handle and data is never freed
                pass # LEAK
    return data, vals

def odim_data_ok(path):
    vals = []
    with h5py.File(path, "r") as hfile:
        data = hfile["dataset1/data1/data"][:]
        d_what = hfile["dataset1/data1/what"]
        for k in ATTR_KEYS & d_what.attrs.keys():
            val = hfile["dataset1/data1/what"].attrs[k]
            vals.append(val)
    return data, vals

def dump_memory():
    snapshot = tracemalloc.take_snapshot()
    stats = snapshot.statistics('lineno')
    for stat in stats[:DUMP_COUNT]:
        print(stat)

def test_h5_bug(path):
    print(h5py.version.info)
    tracemalloc.start()

    print(f"GOOD x{NUM_OPENS}")
    for i in range(NUM_OPENS):
        data, vals = odim_data_ok(path)
    dump_memory()

    print(f"LEAKY x{NUM_OPENS}")
    for i in range(NUM_OPENS):
        data, vals = odim_data_leaky(path)
    dump_memory()

if __name__ == "__main__":
    ATTR_KEYS = {"gain", "BOGUS"} # HDF5 attribute keys we try
    DUMP_COUNT = 3              # number of memory stats to dump
    NUM_OPENS = 10              # number of opens per test fn
    ODIM_PATH = sys.argv[1]     # path to ODIM HDF5 file

    test_h5_bug(ODIM_PATH)
@takluyver
Copy link
Member

Thank-you! Is it possible for you to share a sample file to run that script on? Or even better, a bit of Python code that will create a file like that?

I'm a bit puzzled because the line you point to as allocating the memory is in the dataset read code, not the attributes where you're catching KeyError.

h5py/h5py/_hl/dataset.py

Lines 756 to 758 in 89374b7

if self._fast_read_ok and (new_dtype is None):
try:
return self._fast_reader.read(args)

@tacaswell
Copy link
Member

knee-jerk guess: when attribute access fails we fail to decref the dataset (or something that holds a reference to it). I suspect the thing to look at is to chase through

h5py/h5py/h5a.pyx

Lines 79 to 81 in 4c01efa

if name != NULL:
return AttrID(H5Aopen_by_name(loc.id, obj_name, name,
H5P_DEFAULT, pdefault(lapl)))
and where ever we set KeyError.

A second knee-jerk guess: the stack-trace is keeping things alive.

@bomangus
Copy link
Author

bomangus commented Dec 5, 2023

I'm a bit puzzled because the line you point to as allocating the memory is in the dataset read code, not the attributes where you're catching KeyError.

h5py/h5py/_hl/dataset.py

Lines 756 to 758 in 89374b7

if self._fast_read_ok and (new_dtype is None):
try:
return self._fast_reader.read(args)

What I'm seeing is none of the objects allocated while reading the file are freed once the bug is triggered.
It looks to me like a handle is lost when the KeyError exception is raised.

@bomangus
Copy link
Author

bomangus commented Dec 5, 2023

Thank-you! Is it possible for you to share a sample file to run that script on? Or even better, a bit of Python code that will create a file like that?

Here's a sample ODIM HDF5 file someone kindly uploaded to github:

https://github.com/adokter/ODIM-hdf5-test/blob/master/pvol/bejab_pvol_20151009T0000Z.h5

@takluyver
Copy link
Member

For me (also on Fedora 39) your script seems to work correctly, with no leak 😕

Summary of the h5py configuration
---------------------------------

h5py    3.10.0
HDF5    1.14.2
Python  3.12.0 (main, Oct  2 2023, 00:00:00) [GCC 13.2.1 20230918 (Red Hat 13.2.1-3)]
sys.platform    linux
sys.maxsize     9223372036854775807
numpy   1.26.2
cython (built with) 0.29.36
numpy (built against) 1.26.0
HDF5 (built against) 1.14.2

GOOD x10
/home/takluyver/.local/lib/python3.12/site-packages/h5py/_hl/dataset.py:758: size=106 KiB, count=3, average=35.2 KiB
/home/takluyver/.local/lib/python3.12/site-packages/h5py/_hl/dataset.py:645: size=4624 B, count=1, average=4624 B
/home/takluyver/.local/lib/python3.12/site-packages/h5py/_hl/group.py:357: size=4320 B, count=36, average=120 B
LEAKY x10
/home/takluyver/.local/lib/python3.12/site-packages/h5py/_hl/dataset.py:758: size=106 KiB, count=3, average=35.2 KiB
/home/takluyver/.local/lib/python3.12/site-packages/h5py/_hl/attrs.py:57: size=4624 B, count=1, average=4624 B
/home/takluyver/.local/lib/python3.12/site-packages/h5py/_hl/group.py:357: size=4320 B, count=36, average=120 B

@takluyver
Copy link
Member

Experimenting a bit more: I do see the leak with the Fedora build of h5py:

LEAKY x10
/usr/lib64/python3.12/site-packages/h5py/_hl/dataset.py:758: size=1056 KiB, count=29, average=36.4 KiB
/usr/lib64/python3.12/site-packages/h5py/_hl/attrs.py:56: size=17.0 KiB, count=124, average=141 B
/usr/lib64/python3.12/site-packages/h5py/_hl/group.py:357: size=15.5 KiB, count=78, average=203 B

But if I use pip to install h5py 3.10.0 from a wheel on PyPI, it doesn't leak:

LEAKY x10
/usr/local/lib64/python3.12/site-packages/h5py/_hl/dataset.py:758: size=106 KiB, count=3, average=35.2 KiB
/usr/local/lib64/python3.12/site-packages/h5py/_hl/attrs.py:57: size=4624 B, count=1, average=4624 B
/usr/local/lib64/python3.12/site-packages/h5py/_hl/group.py:357: size=4320 B, count=36, average=120 B

There are a few patches in Fedora, but none of them look obviously relevant. Fedora's packages are built against a newer Cython, but an older NumPy and HDF5, so I guess one of those version differences is probably the key.

@tacaswell
Copy link
Member

I can not reproduce this with my all-the-defaults environment:

Summary of the h5py configuration
---------------------------------

h5py    3.10.0
HDF5    1.14.3
Python  3.12.0+ (heads/3.12:8d21242bd1, Dec  4 2023, 12:44:27) [GCC 13.2.1 20230801]
sys.platform    linux
sys.maxsize     9223372036854775807
numpy   2.0.0.dev0+git20231203.15211ed
cython (built with) 3.1.0a0
numpy (built against) 2.0.0.dev0+git20231203.15211ed
HDF5 (built against) 1.14.3

GOOD x10
/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/h5py/_hl/dataset.py:758: size=106 KiB, count=3, average=35.2 KiB
/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/h5py/_hl/dataset.py:645: size=4624 B, count=1, average=4624 B
/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/h5py/_hl/group.py:357: size=4440 B, count=37, average=120 B
LEAKY x10
/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/h5py/_hl/dataset.py:758: size=106 KiB, count=3, average=35.2 KiB
/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/h5py/_hl/attrs.py:57: size=4624 B, count=1, average=4624 B
/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/h5py/_hl/group.py:357: size=4440 B, count=37, average=120 B

nor with the system packages from Arch.

Summary of the h5py configuration
---------------------------------

h5py    3.10.0
HDF5    1.14.3
Python  3.11.6 (main, Nov 14 2023, 09:36:21) [GCC 13.2.1 20230801]
sys.platform    linux
sys.maxsize     9223372036854775807
numpy   1.26.2
cython (built with) 0.29.36
numpy (built against) 1.26.1
HDF5 (built against) 1.14.3

GOOD x10
/usr/lib/python3.11/site-packages/h5py/_hl/dataset.py:758: size=106 KiB, count=3, average=35.2 KiB
/usr/lib/python3.11/site-packages/h5py/_hl/dataset.py:645: size=4624 B, count=1, average=4624 B
/usr/lib/python3.11/site-packages/h5py/_hl/group.py:357: size=4320 B, count=36, average=120 B
LEAKY x10
/usr/lib/python3.11/site-packages/h5py/_hl/dataset.py:758: size=106 KiB, count=3, average=35.2 KiB
/usr/lib/python3.11/site-packages/h5py/_hl/attrs.py:57: size=4624 B, count=1, average=4624 B
/usr/lib/python3.11/site-packages/h5py/_hl/group.py:357: size=4320 B, count=36, average=120 B

Thank you for an A+ reproducer script + data!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants