Skip to content

fix(vk): reject unbound imported buffers and roll back ownership - #1718

Merged
hckrman101 merged 2 commits into
NotRexed:mainfrom
Arkex-ai:fix/import-buffer-binding
Nov 10, 2026
Merged

hckrman101 merged 2 commits into
NotRexed:mainfrom
Arkex-ai:fix/import-buffer-binding

Conversation

@Arkex-ai

@Arkex-ai Arkex-ai commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A failed vkBindBufferMemory could still be reported as a usable imported buffer. Both host-pointer and non-sparse DMA-buf factories now reject failed binding and roll back the resources they own before publishing an address.

Technical changes

  • Check binding after successful allocation; free memory only when allocation succeeded, then reset failed host-import handles.
  • Preserve main's destroy_import_buffer helper so associated texel views are released before raw buffer destruction.
  • Keep the original binding result, range retry hold, DMA-to-host fallback and sparse-route delegation. No API change or extra GPU wait.

Validation

Head 27d7e7c, based on main 117eae7:

  • 34 cases pass on Windows LLVM and Linux GCC, including ASan/UBSan. 11 mutations are rejected, including omission of the owner-release hook.
  • Full local Windows suite: 298 passed / 2 failed / 21 skipped, versus 297 / 2 / 21 on the exact baseline. The same NUP-path and SDK-header tests fail; the aggregate is not green.
  • The approved factory behavior is unchanged after normalizing main's owner helper. Current-head native gameplay has not been repeated.
Current-head fork check Result
Linux AppImage Passed
Windows build Passed
Source suite Passed
Earlier native evidence and limits

At 2ec6ae3 (API12), serial and parallel/batch checkpoint runs passed with fixed 2x interpolation, resize and six decoded 3440x1440 captures. Both exited normally and preserved the original checkpoint. This covers the healthy Windows host-pointer path; native DMA-buf failure, GPU OOM and campaign coverage are untested.

@Arkex-ai
Arkex-ai force-pushed the fix/import-buffer-binding branch 5 times, most recently from d45f24b to ca857d8 Compare November 10, 2026 17:57
@Arkex-ai

Copy link
Copy Markdown
Contributor Author

Rebased onto main d224dc06; resolved the runner conflict using per-test RUN_ALL_ARGS. The production patch ID is unchanged. Focused imported-buffer ownership and runner-declaration checks pass.

Head ca857d8fd249c8d03265b357bc6a70e1eedc0c21; fresh CIs pending:

Full-suite/current API13 native validation remains separate and pending.

@Arkex-ai

Copy link
Copy Markdown
Contributor Author

Thanks, agreed. I rewrote this and the other four active PR descriptions: the plain-language problem comes first, followed by brief technical changes and one validation section with measured test results and current-head CI links. Rebase history and repeated status lists are removed; earlier native evidence is folded separately so it is not confused with the current head. No code changed.

Check both host-pointer and DMA-buf binding results before publication. Preserve fallback and hold retry semantics while releasing only successfully owned resources at existing cleanup points.

Assisted-by: GPT-6 (OpenAI)
@Arkex-ai
Arkex-ai force-pushed the fix/import-buffer-binding branch from ca857d8 to 27d7e7c Compare November 10, 2026 19:55
@Arkex-ai

Copy link
Copy Markdown
Contributor Author

Rebased onto main 117eae7 at 27d7e7c. The binding checks, rollback conditions, fallback and retry holds are unchanged; both rollback sites retain main's destroy_import_buffer owner-release boundary. The fixture checks that the release hook runs before destruction.

All 34 cases pass on Windows/Linux, including sanitizers; 11 mutations are rejected. Full local suite: 298 passed / 2 failed / 21 skipped, versus 297 / 2 / 21 on the exact baseline, with the same two local failures. Fresh exact-head CIs are linked in the description; current-head native gameplay remains untested.

@hckrman101
hckrman101 merged commit 325583b into NotRexed:main Nov 10, 2026
10 checks passed
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