fix: add thread safety to preload_windows_gl_dlls

[hobostay]

Summary

• Wrap shared mutable state mutations in preload_windows_gl_dlls() with _CONTEXT_LOCK to prevent data races during concurrent envpool.make() calls

Problem

preload_windows_gl_dlls() modifies three module-level mutable collections (_WINDOWS_DLL_HANDLES, _REGISTERED_DLL_DIRS, _PRELOADED_DLL_PATHS) without holding the existing _CONTEXT_LOCK. Concurrent envpool.make() calls that trigger preload_windows_gl_dlls() can corrupt these shared data structures.

Test plan

• Run existing test suite to confirm no regressions

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85a24a2101

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid reacquiring the non-reentrant context lock

On Windows when ENVPOOL_DLL_DIR is set to a valid DLL directory, try_ensure_mujoco_glfw_context() already holds _CONTEXT_LOCK while constructing _GlfwContext, and _GlfwContext.__init__() calls preload_windows_gl_dlls(). This new with _CONTEXT_LOCK: therefore tries to acquire the same threading.Lock a second time in the same thread, causing the first GLFW context initialization to hang instead of rendering or falling back. Use an RLock or move the preload outside the outer locked region.

Useful? React with 👍 / 👎.

[Trinkle23897]

what kind of issue do you want to address? if it's a real issue, could you provide your minimal repo?

[hobostay]

You're right to question this — I think this PR should be closed entirely.

The original code is not buggy. Here's why:

The call chain is:

try_ensure_mujoco_glfw_context()   # acquires _CONTEXT_LOCK
  → _GlfwContext(width, height)
    → _GlfwContext.__init__()
      → preload_windows_gl_dlls()   # already under _CONTEXT_LOCK

preload_windows_gl_dlls() is always called while _CONTEXT_LOCK is already held, so the shared mutable state (_WINDOWS_DLL_HANDLES, _REGISTERED_DLL_DIRS, _PRELOADED_DLL_PATHS) is already protected. There is no data race.

This PR introduces a real P1 deadlock. _CONTEXT_LOCK is a threading.Lock (non-reentrant). Adding with _CONTEXT_LOCK: inside preload_windows_gl_dlls() causes the same thread to acquire it twice → deadlock on the first envpool.make() call on Windows.

I'd recommend closing this PR as-is. If thread safety for standalone preload_windows_gl_dlls() calls (outside the _CONTEXT_LOCK path) is ever needed, the fix would be to change threading.Lock() → threading.RLock(), not to add a nested with _CONTEXT_LOCK.

🤖 Generated with Claude Code

[hobostay]

Closing: the original code has no data race (preload_windows_gl_dlls is always called under _CONTEXT_LOCK via try_ensure_mujoco_glfw_context → _GlfwContext.init), and this PR introduces a deadlock with the non-reentrant threading.Lock.