Skip to content

fix ThreadsafeTypeKeyHashTable.GetOrAdd stampede issue#1577

Merged
AArnott merged 1 commit intoMessagePack-CSharp:masterfrom
gepa21:pr/fix_ThreadsafeTypeKeyHashTable_GetOrAdd
Mar 16, 2023
Merged

fix ThreadsafeTypeKeyHashTable.GetOrAdd stampede issue#1577
AArnott merged 1 commit intoMessagePack-CSharp:masterfrom
gepa21:pr/fix_ThreadsafeTypeKeyHashTable_GetOrAdd

Conversation

@gepa21
Copy link
Contributor

@gepa21 gepa21 commented Mar 15, 2023

GetOrAdd is not running inside a lock, so if multiple threads try to serialize the same type, it could result in the throw new InvalidOperationException("Failed to get or add.");.

for example :

  1. 2 threads enter the function at the same time
  2. both threads fail to Get value and are now in Add attempt
  3. one succeeds (in locked context so the other waits)
  4. second thread fails to add item and throws exception

fix :
after failing to Add() item try to Get it again before raising exception

@gepa21 gepa21 force-pushed the pr/fix_ThreadsafeTypeKeyHashTable_GetOrAdd branch from 77fe01b to 2d5494c Compare March 15, 2023 21:42
Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Wow. Good find. Thank you.

@AArnott AArnott added this to the v2.5 milestone Mar 16, 2023
@AArnott AArnott merged commit c3b4a71 into MessagePack-CSharp:master Mar 16, 2023
@AArnott
Copy link
Collaborator

AArnott commented Mar 16, 2023

It looks like I broke this in a398f71. There's no reason at all to throw here, so I'm going to follow-up with another fix to correct my original mistake. #1579

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