Fix concurrency issues leading to panic in Coffer#167
Fix concurrency issues leading to panic in Coffer#167awnumar merged 9 commits intoawnumar:masterfrom
Conversation
|
Tests have a data race now |
|
The higher concurrency caught a data race in the windows test, we don't want to work around that by just lowering the number of threads. https://github.com/awnumar/memguard/actions/runs/16229178136/job/45830415615#step:6:346 |
|
Looks like the CI runner ran out of mlock capacity and so panicked, which then triggered the global cleanup routines. Having a global shared mutable variables to store pointers to all allocated memory was a big API mistake in hindsight.
This is out of necessity, as we cannot control what callers do with their returned memory once it's given to them. Thanks for your time on this. Please revert the change that adds IsDestroyed, or make the method lowercase at least, as I'm not sure we'll end up keeping that method if we solve the race a different way. Once that's done, I'll merge and iterate separately to this PR. |
There is at least one issue here with lock usage leading to a panic. The code would call
.Destroyed()then take the lock and assume the coffer is not destroyed, when it can be destroyed after the lock is acquired. This caused a panic in the test I added. This has been refactored to take the lock, check the function.destroyed()and then make decisions based off that.