Skip to content

fix: Avoid object cycles to relieve gc pressure #283

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

Merged
tmathern merged 3 commits into from
Jun 11, 2026
Merged

fix: Avoid object cycles to relieve gc pressure #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f5454b
fix: Initial gc investigation
tmathern Jun 10, 2026
ef12fae
fix: Initial gc investigation 2
tmathern Jun 10, 2026
f605566
fix: Debug cleanup
tmathern Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 52 additions & 23 deletions src/c2pa/c2pa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,13 @@ def __init__(self):

@staticmethod
def _free_native_ptr(ptr):
"""Free a native pointer by casting it to c_void_p and calling c2pa_free."""
_lib.c2pa_free(ctypes.cast(ptr, ctypes.c_void_p))
"""Free a native pointer by passing it to c2pa_free.

c2pa_free's argtype is c_void_p, so ctypes converts any pointer
instance directly. (ctypes.cast(ptr, c_void_p) would do the same
conversion but leaves a reference cycle behind on every call.)
"""
_lib.c2pa_free(ptr)

def _ensure_valid_state(self):
"""Raise if the resource is closed or uninitialized."""
Expand Down Expand Up @@ -881,33 +886,49 @@ def __init__(self):
self._data_dir_str = ""


# Wrap a raw (address, length) native region in a writable memoryview
_PyMemoryView_FromMemory = ctypes.pythonapi.PyMemoryView_FromMemory
_PyMemoryView_FromMemory.restype = ctypes.py_object
_PyMemoryView_FromMemory.argtypes = (
ctypes.c_void_p, ctypes.c_ssize_t, ctypes.c_int)
_PyBUF_WRITE = 0x200


def _writable_memoryview(address, length):
return _PyMemoryView_FromMemory(address, length, _PyBUF_WRITE)


def _convert_to_py_string(value) -> str:
if value is None:
return ""

py_string = ""

# Validate pointer before casting and freeing
if not isinstance(value, (int, ctypes.c_void_p)) or value == 0:
# Validate and normalize pointer before reading and freeing.
if isinstance(value, ctypes.c_void_p):
address = value.value
elif isinstance(value, int):
address = value
else:
return ""
if not address:
return ""

try:
ptr = ctypes.cast(value, ctypes.c_char_p)
raw = ctypes.string_at(address)

# Only if we got a valid pointer with valid content
if ptr and ptr.value is not None:
try:
py_string = raw.decode('utf-8', errors='strict')
except Exception:
py_string = ""
finally:
# Only free if we have a valid pointer
try:
py_string = ptr.value.decode('utf-8', errors='strict')
_lib.c2pa_string_free(value)
except Exception:
py_string = ""
finally:
# Only free if we have a valid pointer
try:
_lib.c2pa_string_free(value)
except Exception:
# Ignore clean up issues
pass
except (ctypes.ArgumentError, TypeError, ValueError):
# Ignore clean up issues
pass
except (ctypes.ArgumentError, TypeError, ValueError, OSError):
# Invalid pointer type or value
return ""

Expand Down Expand Up @@ -995,8 +1016,7 @@ def _parse_operation_result_for_error(
if check_error:
error = _lib.c2pa_error()
if error:
error_str = ctypes.cast(
error, ctypes.c_char_p).value.decode('utf-8')
error_str = ctypes.string_at(error).decode('utf-8')
_lib.c2pa_string_free(error)
_raise_typed_c2pa_error(error_str)
return None
Expand Down Expand Up @@ -1617,10 +1637,19 @@ def read_callback(ctx, data, length):
readinto = getattr(stream, "readinto", None)
if readinto is not None:
# Most streams have readinto
buf = (ctypes.c_char * length).from_address(
ctypes.addressof(data.contents))
n = readinto(buf)
return n if n else 0
buf = _writable_memoryview(
ctypes.addressof(data.contents), length)
try:
n = readinto(buf)
finally:
# Invalidate the view:
# The native buffer is only valid for
# the duration of the callback...
buf.release()
if not n:
return 0
# Never report more than the buffer can hold
return min(n, length)

# Fallback for streams without readinto.
buffer = stream.read(length)
Expand Down
2 changes: 2 additions & 0 deletions tests/perf/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ A memory leak grows proportionally with work done. If you sign 50 images and get

The baseline captures this expected static overhead. Future runs compare against it: if `leaked_bytes` grows beyond the baseline by more than 10%, the run fails.

The framework runs `gc.collect()` twice after the scenario finishes, while memray is still tracking. Without that sweep, objects sitting in not-yet-collected reference cycles would be counted in `leaked_bytes` and the number would depend on garbage collector timing rather than on actual leaks. With it, `leaked_bytes` means memory that is still allocated even though nothing in Python can reach it: true leaks plus the one-time static overhead described above.

### How to confirm no leak exists?

Run with a higher iteration count than default (100) and compare:
Expand Down
173 changes: 94 additions & 79 deletions tests/perf/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,139 +2,154 @@
"_meta": {
"memray_version": "1.19.3",
"python_version": "3.12.13",
"c2pa_native_version": "c2pa-v0.85.1",
"c2pa_native_version": "c2pa-v0.86.1",
"iterations": 100,
"perf_env": "python-3.12-slim",
"arch": "aarch64"
},
"reader_jpeg_legacy": {
"peak_bytes": 3814421,
"leaked_bytes": 3266116,
"total_allocations": 698899
"peak_bytes": 3730321,
"leaked_bytes": 3236992,
"total_allocations": 717596
},
"reader_jpeg_with_context": {
"peak_bytes": 3822953,
"leaked_bytes": 3257471,
"total_allocations": 692953
"peak_bytes": 3724412,
"leaked_bytes": 3229219,
"total_allocations": 711543
},
"reader_mp4": {
"peak_bytes": 4876441,
"leaked_bytes": 3257485,
"total_allocations": 2112991
"peak_bytes": 4099225,
"leaked_bytes": 3228160,
"total_allocations": 2084373
},
"reader_wav": {
"peak_bytes": 5520266,
"leaked_bytes": 3267427,
"total_allocations": 400371
"peak_bytes": 4399719,
"leaked_bytes": 3238102,
"total_allocations": 408059
},
"builder_sign_jpeg_legacy": {
"peak_bytes": 7695310,
"leaked_bytes": 3383623,
"total_allocations": 522425
"peak_bytes": 7663441,
"leaked_bytes": 3352658,
"total_allocations": 555922
},
"builder_sign_jpeg_with_context": {
"peak_bytes": 7688236,
"leaked_bytes": 3376293,
"total_allocations": 516851
"peak_bytes": 7656560,
"leaked_bytes": 3345863,
"total_allocations": 550105
},
"builder_sign_png_legacy": {
"peak_bytes": 7932767,
"leaked_bytes": 3383648,
"total_allocations": 1694629
"peak_bytes": 7900956,
"leaked_bytes": 3351994,
"total_allocations": 1978914
},
"builder_sign_png_with_context": {
"peak_bytes": 7925490,
"leaked_bytes": 3376452,
"total_allocations": 1688908
"peak_bytes": 7893973,
"leaked_bytes": 3345450,
"total_allocations": 1973003
},
"builder_sign_jpeg_parallel_split_pool": {
"peak_bytes": 45764159,
"leaked_bytes": 3818113,
"total_allocations": 528785
"peak_bytes": 45726143,
"leaked_bytes": 3714796,
"total_allocations": 557891
},
"builder_sign_jpeg_parallel_split_barrier": {
"peak_bytes": 46225287,
"leaked_bytes": 3809216,
"total_allocations": 527412
"peak_bytes": 45817488,
"leaked_bytes": 3780629,
"total_allocations": 627768
},
"builder_sign_png_parallel_split_pool": {
"peak_bytes": 46002549,
"leaked_bytes": 3817801,
"total_allocations": 1700731
"peak_bytes": 40563556,
"leaked_bytes": 3746819,
"total_allocations": 1984928
},
"builder_sign_png_parallel_split_barrier": {
"peak_bytes": 45970433,
"leaked_bytes": 3812044,
"total_allocations": 1699396
"peak_bytes": 45964496,
"leaked_bytes": 3745249,
"total_allocations": 1983686
},
"builder_sign_gif": {
"peak_bytes": 14544515,
"leaked_bytes": 3375865,
"total_allocations": 7183237
"peak_bytes": 14514114,
"leaked_bytes": 3345545,
"total_allocations": 8547131
},
"builder_sign_heic": {
"peak_bytes": 4608484,
"leaked_bytes": 3376030,
"total_allocations": 771079
"peak_bytes": 7717414,
"leaked_bytes": 3381771,
"total_allocations": 877927
},
"builder_sign_m4a": {
"peak_bytes": 18849082,
"leaked_bytes": 3376431,
"total_allocations": 2273497
"peak_bytes": 18817771,
"leaked_bytes": 3345503,
"total_allocations": 2627261
},
"builder_sign_webp": {
"peak_bytes": 8900701,
"leaked_bytes": 3376432,
"total_allocations": 487683
"peak_bytes": 8869451,
"leaked_bytes": 3345563,
"total_allocations": 496534
},
"builder_sign_avi": {
"peak_bytes": 7040387,
"leaked_bytes": 3376267,
"total_allocations": 40315553
"peak_bytes": 7009007,
"leaked_bytes": 3345266,
"total_allocations": 45029611
},
"builder_sign_mp4": {
"peak_bytes": 6162851,
"leaked_bytes": 3376431,
"total_allocations": 1809672
"peak_bytes": 6131977,
"leaked_bytes": 3345600,
"total_allocations": 1923444
},
"builder_sign_tiff": {
"peak_bytes": 13124728,
"leaked_bytes": 3376268,
"total_allocations": 5139967
"peak_bytes": 13091168,
"leaked_bytes": 3345348,
"total_allocations": 5469122
},
"builder_sign_jpeg_parent_of": {
"peak_bytes": 14173992,
"leaked_bytes": 3377656,
"total_allocations": 1209933
"peak_bytes": 14143351,
"leaked_bytes": 3345698,
"total_allocations": 1285766
},
"builder_sign_jpeg_component_of": {
"peak_bytes": 14175518,
"leaked_bytes": 3377891,
"total_allocations": 1232336
"peak_bytes": 14144869,
"leaked_bytes": 3345779,
"total_allocations": 1308244
},
"builder_sign_jpeg_parent_and_component": {
"peak_bytes": 14530406,
"leaked_bytes": 3474418,
"total_allocations": 2160934
"peak_bytes": 14434957,
"leaked_bytes": 3450621,
"total_allocations": 2289962
},
"builder_sign_jpeg_parent_and_component_mixed_mime": {
"peak_bytes": 14476171,
"leaked_bytes": 3378735,
"total_allocations": 2451587
"peak_bytes": 14445750,
"leaked_bytes": 3345959,
"total_allocations": 2787986
},
"builder_sign_jpeg_two_components_same_mime": {
"peak_bytes": 14519270,
"leaked_bytes": 3473673,
"total_allocations": 2150782
"peak_bytes": 14432257,
"leaked_bytes": 3442393,
"total_allocations": 2279745
},
"builder_sign_jpeg_two_components_mixed_mime": {
"peak_bytes": 14473127,
"leaked_bytes": 3377445,
"total_allocations": 2441195
"peak_bytes": 14443122,
"leaked_bytes": 3346165,
"total_allocations": 2777653
},
"builder_sign_jpeg_archive_roundtrip": {
"peak_bytes": 14226832,
"leaked_bytes": 3426491,
"total_allocations": 1680290
"peak_bytes": 14175766,
"leaked_bytes": 3365189,
"total_allocations": 1767740
},
"reader_error_no_manifest": {
"peak_bytes": 3443163,
"leaked_bytes": 3207515,
"total_allocations": 173242
},
"builder_error_invalid_manifest": {
"peak_bytes": 3243627,
"leaked_bytes": 3186717,
"total_allocations": 95461
},
"reader_string_apis": {
"peak_bytes": 3857136,
"leaked_bytes": 3229581,
"total_allocations": 1178409
}
}
5 changes: 5 additions & 0 deletions tests/perf/run_profile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ def _run_scenario_under_memray(name: str, bin_path: Path) -> None:
sys.path.insert(0, "{repo_root / 'src'}")
from tests.perf.scenarios import SCENARIOS
SCENARIOS["{name}"]({ITERATIONS})
# Collect cycle garbage before tracking ends so leaked_bytes means
# "still allocated but unreachable" (true leaks + one-time statics).
import gc
gc.collect()
gc.collect()
"""
cmd = [
sys.executable, "-m", "memray", "run",
Expand Down
Loading
Loading