[opt]thread safe#40
Conversation
There was a problem hiding this comment.
Thanks for the contribution, but I do not think this PR is safe to merge in its current form.
The intent is good, but the implementation does not actually make the renderer caches thread-safe. It adds method-local mutex guards around VkShaderCache, but several public APIs still return raw pointers or mutable references to internal containers after the lock has already been released:
- VkShaderCache::GetRenderMeta() returns const ShaderRenderMeta* into m_renderMetas.
- VkShaderCache::FindVertCode() and FindFragCode() return const std::vector* into m_vertCodes / m_fragCodes.
- VkTextureCache::Find() returns VkTexture* owned by the internal unique_ptr map.
- VkShaderCache::GetProgramCache() still returns a mutable ShaderProgramCache& without any cache-level synchronization.
Call sites use these returned pointers after the getter has returned, for example material pipeline creation dereferences shader code pointers after FindVertCode() / FindFragCode() has released the mutex. If another thread unloads, clears, or mutates the cache at that time, the caller can still observe a dangling pointer or data race. So this PR may create a false sense of thread safety without fixing the lifetime problem.
add thread safe