Potential AV in _JsonFast

mpv · 2018-12-23 13:50:58

I'm randomly (very rarely, maybe once out of 100,000 calls, but this is very critical) got an AV
After digging deep into code I'm 90% sure this is because of this code

var
  data: variant;
..
  if fUserDataJSON = '' then
    fUserDataJSON := '{}';
  data := _JsonFast(fUserDataJSON);

but the actual problem is inside JSON variants implementation. Function GetVariantFromNotStringJSON( is very "optimistically" in the cases like below

  if (JSON=nil) or
     ((PInteger(JSON)^=NULL_LOW) and (JSON[4] in EndOfJSONValueField)) then

gets access to JSON bytes after JSON[1]

Since this optimization technique is very widely used in mORMot (FALSE_LOW, NULL_LOW etc) IMHO the only possible
way to fix is to allocate a TSynTempBuffer 5 bytes longer than the source string length and fill last 5 bytes with 0

@ab - need your help

Last edited by mpv (2018-12-23 13:53:26)

mpv · 2018-12-27 11:04:20

I create a pull request #168 with a small program to reproduce a problem I was writing about.

This pull request fix only _JsonFast() for cases when TSynTempBuffer buffer if used. More elegant solution is required for other cases (TDocVariantData.InitJSONFromFile for example)

Last edited by mpv (2018-12-27 11:08:11)

ab · 2018-12-27 14:04:56

Please check https://synopse.info/fossil/info/3e501bc977

I hope this fix is wider than your pull request.

mpv · 2018-12-27 15:49:42

Perfect! And with [ac89bf90] even valgrind is happy. Thank you very much

chapa · 2019-01-09 15:29:00

yeah, past three years I experience random AVs as described. One on million, but still random AVs, was unable to track them.

mvp, does it resolves this old timer in your opinion:
https://synopse.info/forum/viewtopic.php?id=3283

Just compiled the new sources, and cross fingers for next 24 hours

ab · 2019-01-09 15:33:39

Current trunk seemed more stable than previous.

I had sometimes random A/V during regression tests under Linux / WSL and not any more.

If it continue to occur, try to disable SSE4.2 StrLen and like in SynCommons.

mpv · 2019-01-09 15:42:54

In my case AV occurs when I use JsonFast and numeric field is in the end of JSON {... "userID":1}, very similar to a case from your code [{.. "Seen": 1}]. Since AB fix it not only for JsonFast but also for TTextWriter I hope it solve your problem

mpv · 2019-01-09 15:59:21

ab wrote:

Current trunk seemed more stable than previous.
I had sometimes random A/V during regression tests under Linux / WSL and not any more.
If it continue to occur, try to disable SSE4.2 StrLen and like in SynCommons.

I think on Windows AV did not occur because FastMM reuse a previously allocated blocks. Isn't it?

chapa · 2019-01-21 09:14:05

@ab I can confirm I get no more random AVs as described!

Thanks once again for the patch.
And to @mvp for finding it, I was unable to and got used to live with it

My build is XE3 CE x64 on various Windows.

ab · 2019-01-21 09:22:09

mORMot Open Source

#1 2018-12-23 13:50:58

Potential AV in _JsonFast

#2 2018-12-27 11:04:20

Re: Potential AV in _JsonFast

#3 2018-12-27 14:04:56

Re: Potential AV in _JsonFast

#4 2018-12-27 15:49:42

Re: Potential AV in _JsonFast

#5 2019-01-09 15:29:00

Re: Potential AV in _JsonFast

#6 2019-01-09 15:33:39

Re: Potential AV in _JsonFast

#7 2019-01-09 15:42:54

Re: Potential AV in _JsonFast

#8 2019-01-09 15:59:21

Re: Potential AV in _JsonFast

#9 2019-01-21 09:14:05

Re: Potential AV in _JsonFast

#10 2019-01-21 09:22:09

Re: Potential AV in _JsonFast

Board footer