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

feat(allocators): RecycleFBA: configurable inherent thread safety #262

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

dnut
Copy link
Contributor

@dnut dnut commented Aug 30, 2024

This implements thread safety within RecycleFBA by acquiring and releasing the lock in the alloc, free, and resize functions. Code that uses the allocator no longer needs to directly access the mutex in the RecycleFBA

Thread safety is configurable using the same approach as the GPA. The type takes a comptime config parameter. Thread safety is on by default if multithreading is supported by the platform.

pinging @dadepo for visibility since he is also using this allocator in his branch.

@dnut dnut requested a review from 0xNineteen August 30, 2024 19:48
@dnut dnut marked this pull request as ready for review August 30, 2024 19:48
previously the init function was broken because it made a copy of the allocator and contained a pointer to the original copy which was discarded from the stack

also, even after fixing that, when GeyserWriter would contain a mutable pointer to itself. the RecycleFBA should really be allocated on the stack. when the data is actually a part of the GeyserWriter struct, it means you have a struct that may be "const" but is still mutable because it contains a mutable pointer to itself. this violates aliasing assumptions and maybe it can trigger UB?
@0xNineteen 0xNineteen merged commit 75c98e6 into main Sep 3, 2024
6 checks passed
@0xNineteen 0xNineteen deleted the dnut/recycle-fba-thread-safe branch September 3, 2024 18:31
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.

3 participants