AllocateVulkanMemory() trashes budget block bytes and has a potential deadlock

[JustSid]

These two issues are only there when running with heap size limits!

In the first case, the m_BlockBytes member is accessed as a pointer, so the compare_and_exchange only has an effect on the first heap. If the allocation comes from a different heap, then it will always be marked as having 0 bytes (which can lead to an underflow later on when the block is freed, which will result in all future allocations failing).

The second case is a deadlock in the compare exchange loop. If the compare exchange fails, it's because another thread modified m_BlockBytes. I'm not 100% sure if this can ever happen or if this is guarded by mutexes, but the compare_and_exchange makes it feel like multiple threads can mess around there. I have not actually ran into this, despite trying, unlike the first issue. Anyways, subsequent iterations will also fail because the stale cached blockBytes value was used.

[adam-sawicki-a]

Regarding 1st problem: You are right. Thanks for catching this mistake! I fixed it.

Regarding 2nd problem: I think you are not right. Please see documentation of atomic compare_exchange* functions: https://en.cppreference.com/w/cpp/atomic/atomic/compare_exchange It says:

Atomically compares the object representation (until C++20)value representation (since C++20) of *this with that of expected, and if those are bitwise-equal, replaces the former with desired (performs read-modify-write operation). Otherwise, loads the actual value stored in *this into expected (performs load operation).

So if other thread modified the value in the meantime, the function returns false, but current value is loaded into blockBytes.

[JustSid]

Ah shoot, you are right! I completely ignored what compare_and_exchange does. Thanks for taking the actual fix though :)