Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: concurrency issue in getDerivedPrimitive #2480

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Conversation

alecthomas
Copy link
Collaborator

@alecthomas alecthomas commented Aug 22, 2024

There is a situation where getDerivedPrimitive can be called concurrently, and two separate keys will be created. This fixes that issue.

Also added a type-safe Mutex type.

There is a situation where getDerivedPrimitive can be called
concurrently, and two separate keys will be created. This fixes that
issue.
@alecthomas alecthomas requested review from gak and a team August 22, 2024 10:48
@ftl-robot ftl-robot mentioned this pull request Aug 22, 2024
@alecthomas alecthomas enabled auto-merge August 22, 2024 10:50
@@ -100,8 +101,7 @@ type KMSEncryptor struct {
root keyset.Handle
kekAEAD tink.AEAD
encryptedKeyset []byte
cachedDerivedMu sync.RWMutex
cachedDerived map[SubKey]tink.AEAD
cachedDerived *mutex.Mutex[map[SubKey]tink.AEAD]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I def read this tink.AEAD as "think ahead"

Comment on lines +11 to +15
// var m mutex.Mutex[*string]
// s := m.Lock()
// defer m.Unlock()
// *s = "hello"
type Mutex[T any] struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, this seems handy! I was going to ask why no use xsync, but this is more generic it seems.

@alecthomas alecthomas added this pull request to the merge queue Aug 22, 2024
Merged via the queue into main with commit bc77ebb Aug 22, 2024
20 checks passed
@alecthomas alecthomas deleted the aat/typesafe-mutex branch August 22, 2024 15:35
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