Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Am I creating a leak here?

I am using the new JsonSerializer from NETCore 3.0's System.Text.Json namespace to deserialize Json documents like this:

var result = JsonSerializer.Deserialize<Response>(json, options);

Response is defined as:

public class Response
{
    public string Foo { get; set; }
    public JsonElement Bar { get; set; }
}

The fact that JsonDocument implements IDisposable makes me wonder if by keeping a reference to an element (Bar) that can be contained in a JsonDocument, will create memory leaks?

Be advised that in general I avoid storing data as kind of "variant" type like this. Unfortunately the structure of the Bar property value is unknown at compile time.

My suspicion stems from System.Text.Json advertised strength's of lazy evaluation and I'm not sure if that involves deferred I/O.

like image 630
Oliver Weichhold Avatar asked Aug 15 '19 06:08

Oliver Weichhold


2 Answers

When you call JsonDocument.Parse it uses pooled arrays to avoid garbage collection pauses in high throughput.

When you Dispose that document it puts the arrays back in the pool, if you lose the reference and it gets garbage collected... it's the same as if it hadn't been disposable at all (slightly worse, since some other aspect of the runtime may suddenly be where things pause and the GC kicks in because the pool has fewer arrays now).

JsonElement has a Clone method (but isn't ICloneable) which returns a copy of the (relevant) data using a JsonDocument instance that doesn't use pooled arrays.

JsonSerializer, when returning JsonElement values, always calls Clone() and then disposes the original JsonDocument. So when you use JsonSerializer and get a JsonElement (directly, or through overflow properties) it's the same as if JsonDocument was built without an ArrayPool optimization... so everything's fine.

like image 106
bartonjs Avatar answered Nov 03 '22 10:11

bartonjs


From a brief investigation of sources (https://github.com/dotnet/corefx/blob/master/src/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs) it seems that JsonDocument Dispose returns "rented" bytes to shared array pool and does some general cleanup. Some instances of JsonDocument are marked as not disposable and in this case Dispose will not do anything. You can check this flag for your instance using reflection - if your instance doesn't have internal IsDisposable flag set to true there is no need to worry, because Dispose will not do anything anyway.

I think in normal scenario, JsonDocument parser should clean after itself and there should be no rented bytes left or internal data after parser is done.

It's always safe to not rely on specific implementation though as it may change and store only references to elements needed. You should probably remap JSON elements to your model, I think that's the whole purpose of JSON deserialization

Quick test:

        var parentField = result.Bar.GetType().GetMember("_parent", MemberTypes.Field, BindingFlags.Instance | BindingFlags.NonPublic)[0] as FieldInfo;
        var parentDocument = parentField.GetValue(result.Bar);

        var isDisposableProperty = parentDocument.GetType().GetProperty("IsDisposable", BindingFlags.Instance | BindingFlags.NonPublic) as PropertyInfo;
        Console.WriteLine(isDisposableProperty.GetValue(parentDocument)); // false

Proves that the instance of JsonDocument held by JsonElement is not disposable.

like image 25
Adassko Avatar answered Nov 03 '22 09:11

Adassko