Get DataContractSerializer to behave nicely with unloadable AssemblyLoadContext #88791
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections; | ||
| using System.Collections.Concurrent; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Reflection; | ||
| using System.Runtime.CompilerServices; | ||
| using System.Runtime.Loader; | ||
| using System.Runtime.Serialization.DataContracts; | ||
|
|
||
| namespace System.Runtime.Serialization | ||
| { | ||
| internal sealed class ContextAwareDataContractIndex | ||
| { | ||
| private (DataContract? strong, WeakReference<DataContract>? weak)[] _contracts; | ||
| private ConditionalWeakTable<Type, DataContract> _keepAlive; | ||
|
|
||
| public int Length => _contracts.Length; | ||
|
|
||
| public ContextAwareDataContractIndex(int size) | ||
| { | ||
| _contracts = new (DataContract?, WeakReference<DataContract>?)[size]; | ||
| _keepAlive = new ConditionalWeakTable<Type, DataContract>(); | ||
| } | ||
|
|
||
| public DataContract? GetItem(int index) => _contracts[index].strong ?? (_contracts[index].weak?.TryGetTarget(out DataContract? ret) == true ? ret : null); | ||
|
|
||
| public void SetItem(int index, DataContract dataContract) | ||
| { | ||
| // Check for unloadability to decide how to store the value | ||
| AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(dataContract.UnderlyingType.Assembly); | ||
| if (alc == null || !alc.IsCollectible) | ||
| { | ||
| _contracts[index].strong = dataContract; | ||
| } | ||
| else | ||
| { | ||
| _contracts[index].weak = new WeakReference<DataContract>(dataContract); | ||
| _keepAlive.Add(dataContract.UnderlyingType, dataContract); | ||
| } | ||
| } | ||
|
|
||
| public void Resize(int newSize) | ||
| { | ||
| Array.Resize<(DataContract?, WeakReference<DataContract>?)>(ref _contracts, newSize); | ||
| } | ||
| } | ||
|
|
||
| internal sealed class ContextAwareDictionary<TKey, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TValue> | ||
| where TKey : Type | ||
| where TValue : class? | ||
| { | ||
| private readonly ConcurrentDictionary<TKey, TValue> _fastDictionary = new(); | ||
| private readonly ConditionalWeakTable<TKey, TValue> _collectibleTable = new(); | ||
|
There was a problem hiding this comment.
|
||
|
|
||
|
|
||
| internal TValue GetOrAdd(TKey t, Func<TKey, TValue> f) | ||
| { | ||
| TValue? ret; | ||
|
|
||
| // The fast and most common default case | ||
| if (_fastDictionary.TryGetValue(t, out ret)) | ||
| return ret; | ||
|
|
||
| // Common case for collectible contexts | ||
| if (_collectibleTable.TryGetValue(t, out ret)) | ||
| return ret; | ||
|
There was a problem hiding this comment. Need to hold the lock on lookup too |
||
|
|
||
| // Not found. Do the slower work of creating the value in the correct collection. | ||
| AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); | ||
|
|
||
| // Null and non-collectible load contexts use the default table | ||
| if (alc == null || !alc.IsCollectible) | ||
| { | ||
| return _fastDictionary.GetOrAdd(t, f); | ||
| } | ||
|
|
||
| // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded | ||
| else | ||
| { | ||
| lock (_collectibleTable) | ||
| { | ||
| if (!_collectibleTable.TryGetValue(t, out ret)) | ||
| { | ||
| ret = f(t); | ||
|
There was a problem hiding this comment. Running the delegate inside the lock is prone to deadlocks. Can you move it outside? There was a problem hiding this comment. The delegate isn't necessarily a simple amount of work depending on your OM design. It's something you really want to avoid executing multiple times. For example, if you instantiate a DCS on an incoming request on a REST api for example, having 100 concurrent requests could result in redoing this expensive work 100 times. You could negatively impact your first request time significantly. There was a problem hiding this comment. You know better if you think that creating a DCS will not execute arbitrary code. But note that There is also There was a problem hiding this comment. I agree, we should place a lock around ConcurrentDictionary.GetOrAdd. We don't need to check a second time if we are always holding a lock when adding as that would guarantee the prior add has completed before calling GetOrAdd and it will act like a Get. |
||
| _collectibleTable.AddOrUpdate(t, ret); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return ret; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does
TKeyhave to be generic? You could just useType.