diff --git a/src/c2pa/c2pa.py b/src/c2pa/c2pa.py index fc62632e..7b7747de 100644 --- a/src/c2pa/c2pa.py +++ b/src/c2pa/c2pa.py @@ -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.""" @@ -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 "" @@ -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 @@ -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) diff --git a/tests/perf/README.md b/tests/perf/README.md index 1f2ec022..cccc60de 100644 --- a/tests/perf/README.md +++ b/tests/perf/README.md @@ -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: diff --git a/tests/perf/baseline.json b/tests/perf/baseline.json index 302d648a..3e436967 100644 --- a/tests/perf/baseline.json +++ b/tests/perf/baseline.json @@ -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 } } \ No newline at end of file diff --git a/tests/perf/run_profile.py b/tests/perf/run_profile.py index 31593967..9b8d4651 100644 --- a/tests/perf/run_profile.py +++ b/tests/perf/run_profile.py @@ -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", diff --git a/tests/perf/scenarios.py b/tests/perf/scenarios.py index 0432aa20..e2945294 100644 --- a/tests/perf/scenarios.py +++ b/tests/perf/scenarios.py @@ -10,12 +10,20 @@ """ import io +import json import os import sys import threading from concurrent.futures import ThreadPoolExecutor from pathlib import Path -from c2pa import Builder, C2paSignerInfo, Context, Reader, Signer +from c2pa import ( + Builder, + C2paError, + C2paSignerInfo, + Context, + Reader, + Signer, +) FIXTURES_DIR = Path(__file__).parent.parent / "fixtures" READING_FIXTURES_DIR = FIXTURES_DIR / "files-for-reading-tests" @@ -452,7 +460,7 @@ def scenario_builder_sign_jpeg_archive_roundtrip(iterations: int = 100) -> None: archive = io.BytesIO() Builder(MANIFEST_BASE).to_archive(archive) archive.seek(0) - # from_archive() yields a context-less Builder; to keep the Context + # from_archive() yields a context-less Builder. To keep the Context # (and its signer), build with the context first, then load the archive. builder = Builder(MANIFEST_BASE, context=context).with_archive(archive) with io.BytesIO(ingredient_bytes) as ing: @@ -463,6 +471,46 @@ def scenario_builder_sign_jpeg_archive_roundtrip(iterations: int = 100) -> None: builder.sign("image/jpeg", io.BytesIO(source_bytes), io.BytesIO()) +def scenario_reader_error_no_manifest(iterations: int = 100) -> None: + """Reader on an unsigned asset: partial-init cleanup.""" + source_bytes = SOURCE_JPEG.read_bytes() # A.jpg carries no manifest + for _ in _iterate(iterations): + try: + Reader("image/jpeg", io.BytesIO(source_bytes)).json() + except C2paError: + pass + + +def scenario_builder_error_invalid_manifest(iterations: int = 100) -> None: + """Error case: Builder with malformed manifest JSON.""" + for _ in _iterate(iterations): + try: + Builder('{"not valid json') + except C2paError: + pass + + +def scenario_reader_string_apis(iterations: int = 100) -> None: + """Uncached string returns: detailed_json/crjson/remote_url/resource_to_stream.""" + source_bytes = SIGNED_JPEG.read_bytes() + context = Context() + # Resolve a real resource URI once, outside the measured loop. + probe = Reader("image/jpeg", io.BytesIO(source_bytes), + manifest_data=None, context=context) + manifests = json.loads(probe.json()) + active = manifests["manifests"][manifests["active_manifest"]] + thumb_uri = active["thumbnail"]["identifier"] + probe.close() + for _ in _iterate(iterations): + reader = Reader("image/jpeg", io.BytesIO(source_bytes), + manifest_data=None, context=context) + reader.detailed_json() + reader.crjson() + reader.get_remote_url() + reader.resource_to_stream(thumb_uri, io.BytesIO()) + reader.close() + + # jpeg + png context variants, paired with the `_legacy` scenarios above for # side-by-side comparison. @@ -524,6 +572,9 @@ def scenario_builder_sign_png_parallel_split_barrier(iterations: int = 100) -> N "builder_sign_jpeg_two_components_same_mime": scenario_builder_sign_jpeg_two_components_same_mime, "builder_sign_jpeg_two_components_mixed_mime": scenario_builder_sign_jpeg_two_components_mixed_mime, "builder_sign_jpeg_archive_roundtrip": scenario_builder_sign_jpeg_archive_roundtrip, + "reader_error_no_manifest": scenario_reader_error_no_manifest, + "builder_error_invalid_manifest": scenario_builder_error_invalid_manifest, + "reader_string_apis": scenario_reader_string_apis, }