Skip to content

fix: clean up tmp_dll_path on integrity verification failure#44

Open
catalini82 wants to merge 1 commit into
ragultv:devfrom
catalini82:fix/integrity-tempfile-cleanup
Open

fix: clean up tmp_dll_path on integrity verification failure#44
catalini82 wants to merge 1 commit into
ragultv:devfrom
catalini82:fix/integrity-tempfile-cleanup

Conversation

@catalini82

Copy link
Copy Markdown

Summary

Fixes a temporary file leak in ensure_binary() when the Windows CUDA DLL archive path fails during download, integrity verification, size validation, or extraction.

The main archive temporary file (tmp_path) was already cleaned up in the exception handler. The separate Windows CUDA DLL archive temporary file (tmp_dll_path) was not, because it is also created with NamedTemporaryFile(delete=False) and needs explicit cleanup.

Changes

oprel/runtime/binaries/installer.py

  • Adds tmp_dll_path cleanup in the except Exception as e: handler.

  • Cleanup is guarded with:

    • "tmp_dll_path" in locals() so failures before DLL temp creation are safe.
    • tmp_dll_path.exists() so already-removed files do not cause errors.
  • Existing tmp_path cleanup is preserved.

  • Original exceptions remain chained through BinaryNotFoundError(... ) from e.

tests/unit/test_download_integrity.py

Adds coverage for:

  • Main archive integrity failure cleans tmp_path.
  • DLL archive integrity failure cleans tmp_dll_path.
  • DLL archive size mismatch cleans tmp_dll_path.
  • Success path with no manifest entry still cleans temporary archives normally.

The Windows CUDA tests monkeypatch get_binary_info() and resolve_version() so they do not depend on current registry constants. Fake downloads route by exact fake URL equality, with an assertion on unexpected URLs.

What did not change

  • No public API changes.
  • No registry URL changes.
  • No manifest entries added.
  • No real or fake checksums added.
  • No config changes.
  • No dependency changes.
  • No success-path download or extraction behavior changes.
  • No extracted directory cleanup added; this PR only handles temporary downloaded archive files.

Validation

  • python3 -m py_compile oprel/runtime/binaries/installer.py
  • python3 -m py_compile oprel/runtime/binaries/integrity.py
  • python3 -m pytest tests/unit/test_download_integrity.py -q -o addopts="" → 33 passed
  • python3 -m pytest tests/unit/test_integrity.py -q -o addopts="" → 33 passed
  • PYTHONPATH=. python3 scripts/smoke_import.py → passed
  • git diff --check → passed

Target branch: dev

@catalini82 catalini82 changed the base branch from main to dev June 29, 2026 14:22
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