Skip to content

Restore count hooks after aborted callbacks - #353

Merged
akeit0 merged 1 commit into
mainfrom
codex/issue338-hook-recovery
Oct 10, 2026
Merged

akeit0 merged 1 commit into
mainfrom
codex/issue338-hook-recovery

Conversation

@akeit0

@akeit0 akeit0 commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator

When a count hook threw, was cancelled, or could not allocate its call frame at the depth limit, IsInHook remained set after the host caught the error. Later executions on the same LuaState silently suppressed hooks, even after reinstalling them. Restore the flag in a finally covering frame setup and callback execution so hooks work when the state is reused.

Add regression coverage for synchronous/asynchronous runtime and host exceptions, cancellation, and frame-depth failure followed by reuse. Also cover callback rearming/replacement/removal, recursive hook suppression, and synchronous/asynchronous host boundaries. Document these behaviors and timeout/memory/error limitations in SetHook API comments and both READMEs.

Refs #338. This PR addresses correctness and documents the observations. The lightweight delegate and first-class budget proposals are deferred; #338 should remain open.

Validation:

  • Runtime tests: 434 passed; Source Generator tests: 69 passed. The initial seven recovery cases failed before the fix.
  • Release build for net10.0/net8.0/net6.0/netstandard2.1: no warnings or errors; CSharpier check passed.
  • Existing internal Unity project, Editor 6000.5.5f1, modern Unity CLI/Pipeline: sync/async exception, cancellation and frame-limit recovery passed; adaptive hook and host-boundary checks passed. Original binaries restored and SHA256 hashes verified. Unity 6000.3/IL2CPP/WebGL remain unverified.
  • BenchmarkDotNet ShortRun, .NET 8.0.31 x64, Release netstandard2.1 assembly, precompiled two-million-iteration numeric loop: before/after no hook 9.208/9.156 ms, empty count=4 27.774/27.745 ms, timestamp+GC count=4 60.816/60.330 ms. No material slowdown observed in this workload; these measurements do not establish general performance improvement.

Temporary probes, experimental APIs and reports are outside Git and are not included in this PR.

@akeit0
akeit0 merged commit 36a08af into main Oct 10, 2026
2 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