#1 2020-04-08 01:17:03

Eugene Ilyin
Member
From: milky_way/orion_arm/sun/earth
Registered: 2016-03-27
Posts: 132
Website

[FIXED] ToJSON, VariantSaveJSON mem leaks for JSONs larger than ~40KB

Hi,

I have an issue with any JSON serialization for content size more than ~40KB.

Minimal code to reproduce:

program ToJSONMemoryLeak;

{$APPTYPE CONSOLE}

uses
  SynCommons;

var
  Index: Integer;
  JSON: RawUTF8;
  V: Variant;

begin
  ReportMemoryLeaksOnShutdown := True;

  // Let's build some DocVariant array of objects [{...}, {...}, ...]
  JSON := '[';
  for Index := 1 to 1400 do
    JSON := JSON + '{"name":"value","prop":false},';
  JSON[Length(JSON)] := ']';

  V := _Json(JSON);

  TDocVariantData(V).ToJSON; // Houston we have a problem
end.

On the recent 1.18.5940 (for both 32-bit and 64-bit platforms) on Delphi 10.3 Rio I have:

Unexpected Memory Leak
An unexpected memory leak has occurred. The sizes of unexpected leaked medium and large blocks are: 16424

Seems like regression in memory management or JSON serialization structures was introduced.

Last edited by Eugene Ilyin (2020-04-09 00:04:51)

Offline

#2 2020-04-08 01:49:14

macfly
Member
From: Brasil
Registered: 2016-08-20
Posts: 374

Re: [FIXED] ToJSON, VariantSaveJSON mem leaks for JSONs larger than ~40KB

I confirm this on Lazarus 2.0.7 + FPC 3.2.

Leak report: https://pastebin.com/dgLEYxGR

Offline

#3 2020-04-08 16:15:37

Eugene Ilyin
Member
From: milky_way/orion_arm/sun/earth
Registered: 2016-03-27
Posts: 132
Website

Re: [FIXED] ToJSON, VariantSaveJSON mem leaks for JSONs larger than ~40KB

Thanks macfly,

The issue is in internal buffer management of TTextWriter.

When the data is bigger than provided external stack-allocated buffer - the TTextWriter code is switching to heap memory allocation
in SynCommons:55649

      if twoBufferIsExternal in fCustomOptions then // use heap, not stack
        exclude(fCustomOptions,twoBufferIsExternal) else
        FreeMem(fTempBuf); // with big content comes bigger buffer
      GetMem(fTempBuf,fTempBufSize);

As you see the twoBufferIsExternal flag is excluded from fCustomOptions to indicate that we must release memory later in TTextWriter.Destroy or during the future internal buffer reallocations.

But! Externally the TDocVariant.ToJSON code restores fCustomOptions from some strange backup hmm variable (?)
As the result twoBufferIsExternal flag is brings back to fCustomOptions and all allocated by the TTextWriter heap memory is never released.

SynCommons:48215

      W.Add(']');
    end;
    W.fCustomOptions := backup;
  end else
Weird solution

Restore from this backup variable all flags except twoBufferIsExternal.

    W.fCustomOptions := backup - [twoBufferIsExternal] + W.fCustomOptions * [twoBufferIsExternal];
Better solution

Refactor TDocVariant.ToJSON code and not use the protected section of TTextWriter class by the external TDocVariant class.

As for now any JSON requests, serialization, or data transfer larger than ~40KB (~4Kb gzip) provides memory leaks on server sad

Last edited by Eugene Ilyin (2020-04-09 00:02:50)

Offline

#4 2020-04-08 23:19:37

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,661
Website

Re: [FIXED] ToJSON, VariantSaveJSON mem leaks for JSONs larger than ~40KB

Oups!
It was something weird indeed.
Thanks a lot for debugging it!

Please check https://synopse.info/fossil/info/b8528d8633
It shouldn't leak memory any more.

Offline

#5 2020-04-09 00:02:14

Eugene Ilyin
Member
From: milky_way/orion_arm/sun/earth
Registered: 2016-03-27
Posts: 132
Website

Re: [FIXED] ToJSON, VariantSaveJSON mem leaks for JSONs larger than ~40KB

Thanks ab,

All allocated memory now released property after the 1.18.5944 release.

Offline

#6 2020-04-09 07:52:26

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,661
Website

Re: [FIXED] ToJSON, VariantSaveJSON mem leaks for JSONs larger than ~40KB

I have added a regression test for TDocVariantData.ToJSON/TTextWriter with huge JSON, to detect any memory leak as reported earlier.

Check https://synopse.info/fossil/info/f9522171d3

Offline

#7 2020-04-09 14:07:08

Eugene Ilyin
Member
From: milky_way/orion_arm/sun/earth
Registered: 2016-03-27
Posts: 132
Website

Re: [FIXED] ToJSON, VariantSaveJSON mem leaks for JSONs larger than ~40KB

Good point!

I was thinking how to integrate this check and prevent such leaks in future and remembered, that SynSelfTests check for mem leaks on complete.

Thanks

Offline

Board footer

Powered by FluxBB