Skip to content

fix(audio2x-common): close file handles and handle missing trtexec in… - #8

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:sweep/bugfix-trt-cache-path
Open

fix(audio2x-common): close file handles and handle missing trtexec in…#8
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:sweep/bugfix-trt-cache-path

Conversation

@andrewwhitecdw

Copy link
Copy Markdown

… get_trt_cache_path

Summary

get_trt_cache_path() in audio2x-common/scripts/audio2x/data_utils.py (used to hash the ONNX model + trtexec binary + args into a TRT engine cache key) leaks two file handles, and crashes with a confusing TypeError: expected str, bytes or os.PathLike object, not NoneType when trtexec is not on PATH — a common situation since the TRT cache is an opt-in development speedup (A2X_SDK_USE_TRT_CACHE=true).

Root cause

digest.update(open(onnx_model_fpath, "rb").read())          # handle never closed
digest.update(open(shutil.which("trtexec"), "rb").read())   # which() may return None -> TypeError

shutil.which() returns None when the executable is not found, and open(None, "rb") raises TypeError, masking the actual problem (trtexec missing).

Fix

Read both files with with open(...) so handles are always closed, and raise a descriptive FileNotFoundError when shutil.which("trtexec") returns None.

Testing

Exercised get_trt_cache_path directly with uv run --with numpy (ephemeral env) against a fake network.onnx and a fake trtexec on PATH:

  • hash is deterministic for identical inputs: OK
  • changing args after the first 3 (trtexec/onnx/output paths) changes the hash: OK
  • with trtexec removed from PATH:
    • after fix: raises FileNotFoundError("trtexec not found in PATH...")
    • before fix (verified with the change stashed): raises TypeError: expected str, bytes or os.PathLike object, not NoneType, confirming the bug is pre-existing

⚠️ The project's full C++/CUDA test suite cannot run locally (requires CUDA + TensorRT + generated test data); this change is confined to a pure-Python helper that was tested in isolation.

Why existing tests missed it

The TRT cache path is only exercised when A2X_SDK_USE_TRT_CACHE=true, which the header comment marks as an opt-in development speedup, and the missing-trtexec case only occurs on machines without TensorRT installed.

@andrewwhitecdw

Copy link
Copy Markdown
Author

Closing this sweep-generated PR: PR has 2 commits; sweep requires exactly one commit per PR. It does not meet the sweep requirements (single signed-off commit).

… get_trt_cache_path

## Summary

`get_trt_cache_path()` in `audio2x-common/scripts/audio2x/data_utils.py`
(used to hash the ONNX model + trtexec binary + args into a TRT engine
cache key) leaks two file handles, and crashes with a confusing
`TypeError: expected str, bytes or os.PathLike object, not NoneType` when
`trtexec` is not on `PATH` — a common situation since the TRT cache is an
opt-in development speedup (`A2X_SDK_USE_TRT_CACHE=true`).

## Root cause

```python
digest.update(open(onnx_model_fpath, "rb").read())          # handle never closed
digest.update(open(shutil.which("trtexec"), "rb").read())   # which() may return None -> TypeError
```

`shutil.which()` returns `None` when the executable is not found, and
`open(None, "rb")` raises `TypeError`, masking the actual problem (trtexec
missing).

## Fix

Read both files with `with open(...)` so handles are always closed, and
raise a descriptive `FileNotFoundError` when `shutil.which("trtexec")`
returns `None`.

## Testing

Exercised `get_trt_cache_path` directly with `uv run --with numpy`
(ephemeral env) against a fake `network.onnx` and a fake `trtexec` on
`PATH`:

- hash is deterministic for identical inputs: OK
- changing args after the first 3 (trtexec/onnx/output paths) changes the
  hash: OK
- with `trtexec` removed from `PATH`:
  - after fix: raises `FileNotFoundError("trtexec not found in PATH...")`
  - before fix (verified with the change stashed): raises
    `TypeError: expected str, bytes or os.PathLike object, not NoneType`,
    confirming the bug is pre-existing

⚠️ The project's full C++/CUDA test suite cannot run locally (requires
CUDA + TensorRT + generated test data); this change is confined to a
pure-Python helper that was tested in isolation.

## Why existing tests missed it

The TRT cache path is only exercised when `A2X_SDK_USE_TRT_CACHE=true`,
which the header comment marks as an opt-in development speedup, and the
missing-`trtexec` case only occurs on machines without TensorRT installed.

Signed-off-by: Andrew White <andrewwhitecdw@users.noreply.github.com>
Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
@andrewwhitecdw
andrewwhitecdw force-pushed the sweep/bugfix-trt-cache-path branch from b405fc4 to 61659e0 Compare August 18, 2026 00:21
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

Successfully merging this pull request may close these issues.

1 participant