Skip to content

Commit 8e3956d

Browse files
committed
Fix possible cycle dead lock.
1 parent 19d5d58 commit 8e3956d

File tree

1 file changed

+33
-67
lines changed

1 file changed

+33
-67
lines changed

src/MsgPack/Serialization/SerializationContext.cs

Lines changed: 33 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ public static SerializationContext Default
116116
private readonly ConcurrentDictionary<Type, object> _typeLock;
117117
#endif // SILVERLIGHT || NETFX_35 || UNITY
118118

119+
private readonly object _generationLock;
120+
119121
/// <summary>
120122
/// Gets the current <see cref="SerializerRepository"/>.
121123
/// </summary>
@@ -544,6 +546,7 @@ public SerializationContext( PackerCompatibilityOptions packerCompatibilityOptio
544546
#else
545547
this._typeLock = new ConcurrentDictionary<Type, object>();
546548
#endif // SILVERLIGHT || NETFX_35 || UNITY
549+
this._generationLock = new object();
547550
this._defaultCollectionTypes = new DefaultConcreteTypeRepository();
548551
#if !XAMIOS &&!UNITY
549552
this._generatorOption = SerializationMethodGeneratorOption.Fast;
@@ -618,58 +621,39 @@ public MessagePackSerializer<T> GetSerializer<T>( object providerParameter )
618621
return serializer;
619622
}
620623

621-
object aquiredLock = null;
622624
bool lockTaken = false;
623-
try
625+
lock ( this._generationLock )
624626
{
625-
try { }
626-
finally
627+
// Re-get to check because other thread might create the serializer when I wait the lock.
628+
serializer = this._serializers.Get<T>( this, providerParameter );
629+
630+
if ( serializer != null )
627631
{
628-
var newLock = new object();
629-
#if SILVERLIGHT || NETFX_35 || UNITY
630-
Monitor.Enter( newLock );
631-
try
632+
return serializer;
633+
}
634+
635+
try
636+
{
637+
try {}
638+
finally
632639
{
640+
#if SILVERLIGHT || NETFX_35 || UNITY
633641
lock ( this._typeLock )
634642
{
643+
var typeLock = new object();
644+
object aquiredLock;
635645
lockTaken = !this._typeLock.TryGetValue( typeof( T ), out aquiredLock );
636646
if ( lockTaken )
637647
{
638-
aquiredLock = newLock;
639-
this._typeLock.Add( typeof( T ), newLock );
648+
this._typeLock.Add( typeof( T ), typeLock );
640649
}
641650
}
642651
#else
643-
bool newLockTaken = false;
644-
try
645-
{
646-
Monitor.Enter( newLock, ref newLockTaken );
647-
aquiredLock = this._typeLock.GetOrAdd( typeof( T ), _ => newLock );
648-
lockTaken = newLock == aquiredLock;
652+
var typeLock = new object();
653+
var aquiredTypeLock = this._typeLock.GetOrAdd( typeof( T ), _ => typeLock );
654+
lockTaken = typeLock == aquiredTypeLock;
649655
#endif // if SILVERLIGHT || NETFX_35 || UNITY
650656
}
651-
finally
652-
{
653-
#if SILVERLIGHT || NETFX_35 || UNITY
654-
if ( !lockTaken )
655-
#else
656-
if ( !lockTaken && newLockTaken )
657-
#endif // if SILVERLIGHT || NETFX_35 || UNITY
658-
{
659-
// Release the lock which failed to become 'primary' lock.
660-
Monitor.Exit( newLock );
661-
}
662-
}
663-
}
664-
665-
if ( Monitor.TryEnter( aquiredLock ) )
666-
{
667-
// Decrement monitor counter.
668-
Monitor.Exit( aquiredLock );
669-
670-
#if DEBUG && !NETFX_40 && !NETFX_35 && !SILVERLIGHT && !UNITY
671-
Contract.Assert( Monitor.IsEntered( aquiredLock ), "Monitor.IsEntered(aquiredLock)" );
672-
#endif // DEBUG && !NETFX_40 && !NETFX_35 !SILVERLIGHT && && !UNITY
673657

674658
if ( lockTaken )
675659
{
@@ -700,9 +684,6 @@ public MessagePackSerializer<T> GetSerializer<T>( object providerParameter )
700684
else
701685
{
702686
// This thread owns existing lock -- thus, constructing self-composite type.
703-
704-
// Prevent release owned lock.
705-
aquiredLock = null;
706687
return new LazyDelegatingMessagePackSerializer<T>( this, providerParameter );
707688
}
708689

@@ -749,7 +730,7 @@ out nullableSerializerProvider
749730
provider,
750731
nullableType,
751732
nullableSerializerProvider,
752-
SerializerRegistrationOptions.WithNullable
733+
SerializerRegistrationOptions.WithNullable
753734
);
754735
#else
755736
this._serializers.Register(
@@ -760,40 +741,25 @@ out nullableSerializerProvider
760741
SerializerRegistrationOptions.None
761742
);
762743
#endif // !UNITY
763-
}
764-
else
765-
{
766-
// Wait creation by other thread.
767-
// Acquire as 'waiting' lock.
768-
Monitor.Enter( aquiredLock );
769-
}
770744

771-
// Re-get to avoid duplicated registration and handle provider parameter or get the one created by prececing thread.
772-
// If T is null and schema is not provided or default schema is provided, then exception will be thrown here from the new provider.
773-
return this._serializers.Get<T>( this, providerParameter );
774-
}
775-
finally
776-
{
777-
if ( lockTaken )
745+
// Re-get to avoid duplicated registration and handle provider parameter or get the one created by prececing thread.
746+
// If T is null and schema is not provided or default schema is provided, then exception will be thrown here from the new provider.
747+
return this._serializers.Get<T>( this, providerParameter );
748+
}
749+
finally
778750
{
751+
if ( lockTaken )
752+
{
779753
#if SILVERLIGHT || NETFX_35 || UNITY
780754
lock ( this._typeLock )
781755
{
782756
this._typeLock.Remove( typeof( T ) );
783757
}
784758
#else
785-
object dummy;
786-
this._typeLock.TryRemove( typeof( T ), out dummy );
759+
object dummy;
760+
this._typeLock.TryRemove( typeof( T ), out dummy );
787761
#endif // if SILVERLIGHT || NETFX_35 || UNITY
788-
}
789-
790-
if ( aquiredLock != null )
791-
{
792-
// Release primary lock or waiting lock.
793-
Monitor.Exit( aquiredLock );
794-
#if DEBUG && !NETFX_40 && !NETFX_35 && !SILVERLIGHT && !UNITY
795-
Contract.Assert( !Monitor.IsEntered( aquiredLock ), "!Monitor.IsEntered(aquiredLock)" );
796-
#endif // DEBUG && !NETFX_40 && !NETFX_35 && !SILVERLIGHT && !UNITY
762+
}
797763
}
798764
}
799765
}

0 commit comments

Comments
 (0)