Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle versioning #308

Open
RossNordby opened this issue Feb 11, 2024 · 0 comments
Open

Handle versioning #308

RossNordby opened this issue Feb 11, 2024 · 0 comments

Comments

@RossNordby
Copy link
Member

It's been on the to-do list for a long time, so it deserves an issue!

Handles aren't versioned. A handle associated with a removed body or constraint could later refer to a different body or constraint.

This opens the door to a lot of use-after-free style bugs. Functions like ConstraintExists are also confusing as a result.

Using a simple low bitcount counter would suffice to catch many cases of misuse (even a single bit would help), but it would not be good enough to make ConstraintExists reliable without expanding the bittedness. A large, long running simulation could easily use more than 16 bits of versioning.

In other words, if you add 2^N constraints over the course of a simulation, you need 2^N unique handles, and that requires N bits. A simulation that creates 1024 new constraints per frame (which is well within realistic limits) would have less than a day of uptime before things like ConstraintExists could potentially fail with 32 bit handles.

Bumping up to 64 bit handles trivially lets every handle value have 32 bits of versioning. That's probably enough for nonpathological use cases, but one could still add/remove billions of times fairly quickly. Going even further out to 64 bits of versioning would make even pathological uses a non-issue, but makes the user-held handles awkwardly sized. Doing something like having a single counter for all constraint handles rather than using separate versioning bits would share the space well enough to stay in 64 bits, but we need low magnitude handles for a lot of the direct mapping stuff. You could still have that, it would just require some kind of additional work (e.g. conceptually, a hash map would suffice).

Having a low-bittedness counter that is allowed to roll over is a lot simpler and keeps the weight of handles down. If we stay with that strategy, things like ConstraintExists should probably be documented/named better to avoid the (reasonable) assumption that handles are not vulnerable to reuse.

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

No branches or pull requests

1 participant