Skip to content

[opt]thread safe#40

Open
piercashuang wants to merge 1 commit into
InfernuxEngine:masterfrom
piercashuang:opt]-add-mutex-protection
Open

[opt]thread safe#40
piercashuang wants to merge 1 commit into
InfernuxEngine:masterfrom
piercashuang:opt]-add-mutex-protection

Conversation

@piercashuang
Copy link
Copy Markdown

add thread safe

@ChenlizheMe ChenlizheMe self-requested a review May 11, 2026 07:37
Copy link
Copy Markdown
Collaborator

@ChenlizheMe ChenlizheMe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants