Skip to content

pkg/cdi: fix, and cleanup some locks, and use RWMutex - #345

Open
thaJeztah wants to merge 4 commits into
cncf-tags:mainfrom
thaJeztah:cdi_locking_cleanups
Open

pkg/cdi: fix, and cleanup some locks, and use RWMutex#345
thaJeztah wants to merge 4 commits into
cncf-tags:mainfrom
thaJeztah:cdi_locking_cleanups

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

pkg/cdi: reduce lock scope in InjectDevices

InjectDevices only needs to hold the lock while refreshing the cache and
capturing the current devices map. Release the lock before resolving devices
to reduce lock contention while applying the container edits on the passed-in
OCI spec.

pkg/cdi: newCache: remove redundant locking and simplify

This is a constructor, working on a freshly created Cache, so there
should be no concurrency to account for. While updating, also set the
default specDirs directly from a clone of DefaultSpecDirs, instead of
applying through WithSpecDirs to avoid the extra indirect.

Paths are still cleaned, to account for the DefaultSpecDirs being
a mutable, package-level variable (but generally not recommended to
update).

pkg/cdi: fix error collection in device cache

Rename vars that shadowed imports, and make sure that the returned errors
are properly dereferenced. Also append directory errors instead of overwriting
existing errors for the same path.

pkg/cdi: Cache: use RWMutex

InjectDevices only needs to hold the lock while refreshing the cache and
capturing the current devices map. Release the lock before resolving devices
to reduce lock contention while applying the container edits on the passed-in
OCI spec.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is a constructor, working on a freshly created `Cache`, so there
should be no concurrency to account for. While updating, also set the
default specDirs directly from a clone of `DefaultSpecDirs`, instead of
applying through `WithSpecDirs` to avoid the extra indirect.

Paths are still cleaned, to account for the `DefaultSpecDirs` being
a mutable, package-level variable (but generally not recommended to
update).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Rename vars that shadowed imports, and make sure that the returned errors
are properly dereferenced. Also append directory errors instead of overwriting
existing errors for the same path.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztah force-pushed the cdi_locking_cleanups branch from 19a854e to e2744b6 Compare August 24, 2026 12:32
@thaJeztah

Copy link
Copy Markdown
Contributor Author

cc @marquiz @klihub ptal 🤗

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.

1 participant