-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
DotNetXmlSerializer leaks memory #1988
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
Labels
Comments
We will use in our project for now this custom class public class DotNetXmlSerializerWithCache : IXmlSerializer
{
private static readonly Dictionary<(Type, string?), XmlSerializer> cache = new();
private static readonly ReaderWriterLockSlim cacheLock = new();
...
/// <summary>
/// Serialize the object as XML
/// </summary>
/// <param name="obj">Object to serialize</param>
/// <returns>XML as string</returns>
public string Serialize(object obj)
{
var ns = new XmlSerializerNamespaces();
ns.Add(string.Empty, this.Namespace);
var serializer = GetXmlSerializer(obj.GetType(), this.RootElement);
var writer = new EncodingStringWriter(this.Encoding);
serializer.Serialize(writer, obj, ns);
return writer.ToString();
}
private static XmlSerializer GetXmlSerializer(Type type, string? rootElement)
{
// We use a read write lock so that the read of an already created instance
// is fast.
// We don't use ConcurrentDictionary because the GetOrAdd method does
// not guarantee that only one instance would be created.
XmlSerializer? serializer = null;
var key = (type, rootElement);
cacheLock.EnterReadLock();
try
{
if (cache.ContainsKey(key))
{
serializer = cache[key];
}
}
finally
{
cacheLock.ExitReadLock();
}
if (serializer != null)
{
return serializer;
}
cacheLock.EnterWriteLock();
try
{
// check again for a cached instance, because between the EnterWriteLock
// and the last check, some other thread could have added an instance
if (!cache.ContainsKey(key))
{
var root = rootElement == null ? null : new XmlRootAttribute(rootElement);
cache[key] = new XmlSerializer(type, root);
}
serializer = cache[key];
}
finally
{
cacheLock.ExitWriteLock();
}
return serializer;
}
} which is a copy of |
Can please someone from the Maintainers respond. Or should I just create a PR for the proposed fix? |
Please submit a PR |
alexeyzimarev
added a commit
that referenced
this issue
Mar 29, 2023
Serializers cache (fixes #1988)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
DO NOT USE ISSUES FOR QUESTIONS
Describe the bug
The default xml-serializer
DotNetXmlSerializer
leaks memory in the form of dynamic assemblies.As stated in the msdn for
XmlSerializer
, each instance ofXmlSerializer
should be cached for later use if it was not created by one of the mentioned self-caching constructors.RestSharp/src/RestSharp/Serializers/Xml/DotNetXmlSerializer.cs
Line 56 in 8fdbb44
To Reproduce
Run
and start a memory profiler or take a memory dump after sometime.
Expected behavior
Reuse serializers.
Stack trace
Copy the full stack trace here if you get an exception.
Desktop (please complete the following information):
Additional context
Due to this memory-leak and a high amount of requestst we have a kubernetes hosted application that increases its memory consumtion by ~3GB in just 6 hours.
We will likly fix this issue our self by using a custom serilizer that includes caching.
The text was updated successfully, but these errors were encountered: