Skip to content

Acquire loader_lock in loader_layer_create/destroy_device - #2055

Closed
mahkoh wants to merge 1 commit into
KhronosGroup:mainfrom
mahkoh:jorth/create-device-lock
Closed

mahkoh wants to merge 1 commit into
KhronosGroup:mainfrom
mahkoh:jorth/create-device-lock

Conversation

@mahkoh

@mahkoh mahkoh commented Oct 11, 2026

Copy link
Copy Markdown

These functions modify the logical_device_list which is protected only by the loader_lock. These functions are passed to layers as callbacks with no restrictions on when to call them. Therefore, these functions must acquire the loader_lock.

Since 5ee27b3 the lock is recursive, which means that acquiring it in these functions will not deadlock even if the callbacks are called while the loader is already holding the lock in the current thread.

Fixes #2052 by fully serializing device creation and destruction.

These functions modify the logical_device_list which is protected only
by the loader_lock. These functions are passed to layers as callbacks
with no restrictions on when to call them. Therefore, these functions
must acquire the loader_lock.

Since 5ee27b3 the lock is recursive,
which means that acquiring it in these functions will not deadlock even
if the callbacks are called while the loader is already holding the lock
in the current thread.

Fixes KhronosGroup#2052 by fully serializing device creation and destruction.

Signed-off-by: Julian Orth <ju.orth@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 00:28
@ci-tester-lunarg

Copy link
Copy Markdown

User mahkoh not on autobuild list. Waiting for curator authorization before starting CI build.

1 similar comment
@ci-tester-lunarg

Copy link
Copy Markdown

User mahkoh not on autobuild list. Waiting for curator authorization before starting CI build.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new cross-thread serialization behavior lacks regression coverage.

1 open finding
What changed in this PR

Serializes layer-driven device creation and destruction with the recursive global loader lock, preventing logical-device list races and dispatch-pointer reuse ordering issues.

Changes:

  • Locks loader_layer_create_device.
  • Locks loader_layer_destroy_device.
File Description
loader/​loader.c Protects device lifecycle callbacks with loader_lock.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread loader/loader.c
struct loader_device *dev = NULL;
struct loader_instance *inst = NULL;

loader_platform_thread_lock_mutex(&loader_lock);
@mahkoh mahkoh closed this Oct 11, 2026
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.

Guarantees regarding dispatch table pointer reuse

4 participants