Repository navigation
Fix concurrent publication of generated LuaObject metatables - #348
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Concurrent first access to generated
[LuaObject]metatables can expose an incomplete table even when callers use separateLuaStateand object instances. The getter currently publishes the table before adding its metamethods, and competing initializers also mutate the shared field's current table.Build each candidate locally, then publish it with
Interlocked.CompareExchangeafter all metamethods are installed. UseVolatile.Read/Writefor the shared property's access. Readers converge on the published instance, and initialization does not overwrite an existing non-null custom metatable. Explicit replacement and null-triggered reinitialization retain their existing behavior. Generated comments explain why publication occurs last.Fixes #309.
Validation:
ExpectedFailure, matching CI. Source Generator tests: 1 passed.816a8a6.Performance measured with BenchmarkDotNet 0.14.0, Windows x64/.NET 8.0.31, Release, two launches, five warmups, twelve measured iterations per launch. Both versions use the same main runtime; only the generator differs. The Lua workload performs 100,000 iterations of property reads and method calls and checks its result.
No regression was observed in these measurements; the Lua workload took approximately 4.7% less time. Both cached-getter measurements have bimodal distributions, so their small absolute difference should not be treated as a reliable speedup. Unity Mono/IL2CPP execution was not measured.
The initialized getter takes no lock. Concurrent cold access can allocate losing candidate tables, which are discarded. This fix makes initial construction/publication safe; it does not synchronize subsequent mutations of a shared metatable or concurrent use of one LuaState.
Temporary benchmark sources, consumer projects, and logs remain outside the repository. This PR is left unmerged for the maintainer's final decision.