onryldz / x-superobject

Delphi Cross Platform Rapid JSON
313 stars 117 forks source link

RTTI and MultiThreading #141

Open CaptainNemo07 opened 3 years ago

CaptainNemo07 commented 3 years ago

Because of RTTI up to 10.3 Rio was not thread-safe (and I do not know about 10.4), the SuperObject's code, which using RTTI must be enveloped by CriticalSection Enter/Leave. Also, cxt:=TRTTIContext.Create may be ran one time. So, my propousal is as follows:

  1. Declare global variable Ctx, initialize it one time in Initialization section and free it in Finalization section.
  2. All uses of RTTIContext via create-use-free may be switched to that variable.
  3. FContext in TGenericsInfo class (line 507) may be deleted, no one different context may be obtained. Classes information was created by the compiler one time, there is no other one.
  4. All the code, which using RTTI context must be enveloped by CriticalSection. In any model, with global Ctx or with create-use-free local vars, it must be made, for avoiding AccessViolation or empty field names and values issues.
vkrapotkin commented 3 years ago

I didn't investigate deeply but all my applications use the XSO lib in threads and never faced with such problems.

CaptainNemo07 commented 3 years ago

I'm many times faced with strange behaviour in multithreading environment with complex objects converting from/to json. Five running threads and TJSON.SuperObject(Item), SuperRecord.FromJSON(Data) give unforgettable feelings.

Tavel commented 2 years ago

I didn't investigate deeply but all my applications use the XSO lib in threads and never faced with such problems.

@vkrapotkin, I made "as simple as possible" demo project to reproduce synchronization errors in XSO during TJSON.Parse call: https://github.com/Tavel/XSOMultithreadingBug

Issue is reproduced both in Delphi 10.4 and Delphi XE7 with the latest available XSuperObject sources.

After project run you can press button to generate 50 (by default) threads which will call TJSON.Parse for defined custom type from several ready JSON strings. Almost immediately after start you will see a lot of access violations and in the folder with SO_sync_bug.exe executable you will find files called "Thread #1_Exception.log", "Thread #2_Exception.log", etc. with a lot of logged EArgumentOutOfRangeException and EAccessViolation catched deeply inside XSO. In some cases you will also find stack traces, which can be helpful to understand where the problem is actually located.

By the way, I love simple and clear syntax of XSuperObject, thank you for this great project!

csm101 commented 1 year ago

I am having the very same problem: in multithreaded applications concurrent attempts to deserialize json to classes will throw access violations and, with my example, quite a lot of EArgumentOutOfRange exceptions.

This is a screenshot of a test application that tries to deserialize 50 identical json objects in in parallel: on average half of the threads will fail.

image

I am running it on a ryzen 5950 cpu which can run up to 32 threads simultaneously, so this might make these errors happen more likely than on other machines.

To me this is a big no-no for using this library in a production environment, especially in a REST service. It's a pity because I like a lot how it is structured and the cleanliness of the code

By the way: this happens with the latest delphi Alexandria 11 update 2.

I am pasting here the source code (pas and dfm) of the form in the above image (VCL/Win32).

Please notice that all the exceptions you see in the image are captured by a try/except that surrounds just the line of code that contains the TJson.Parse call

               try
                 rec := TJSON.Parse<TExternalRecord>(json);  // this raises random exceptions!
              except
                 on e:exception do begin
                   TInterlocked.Increment(errorCount);
                   WriteLog(e.ClassName + ': '+e.Message);
                 end;
              end;

TheFormU.pas

unit TheFormU;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Vcl.StdCtrls, Vcl.ExtCtrls;

type
  TTheForm = class(TForm)
    Panel_78E65794: TPanel;
    ctrlNThreads: TEdit;
    btnBoom: TButton;
    lblNThreads: TLabel;
    Label_4F53C536: TLabel;
    lblThreadCount: TLabel;
    log: TMemo;
    lblErrors: TLabel;
    Label_7E8DB5CF: TLabel;
    procedure btnBoomClick(Sender: TObject);
  private
    procedure WriteLog(msg: string);

    { Private declarations }
  public
    { Public declarations }
  end;

var
  TheForm: TTheForm;

implementation
uses System.SyncObjs,XSuperObject;

{$R *.dfm}

type
  TExternalRecord = record
  public
     type
        TMyArrayElementType = record
          objProperty : string;
        end;

      var
         InnerArrayOfObjects: array of TMyArrayElementType;
  end;

const jsonText:string =
   '{'                              + #13#10 +
    '  "InnerArrayOfObjects":'      + #13#10 +
    '  ['                           + #13#10 +
    '    {'                         + #13#10 +
    '      "objProperty":"MYAPP",'  + #13#10 +
    '    }'                         + #13#10 +
    '  ],'                          + #13#10 +
    '}';

procedure TTheForm.WriteLog(msg:string);
begin
  TThread.Queue(nil,
    procedure
    begin
       log.Lines.Add(msg);
    end) ;
end;

procedure TTheForm.btnBoomClick(Sender: TObject);
var allThreads:TArray<TThread>;
    activeThreadsCount: integer;
    errorCount:integer;
    t:TThread;
    c:integer;
begin
  log.Lines.Clear;

  errorCount := 0;
  activeThreadsCount := StrToIntDef(ctrlNThreads.Text,50);

  lblNThreads.Caption := '--';
  lblErrors.Caption := '--';

  Writelog('Running...');
  btnBoom.Enabled := false;
  try
    SetLength(allThreads,activeThreadsCount);
    for c:= 0 to activeThreadsCount-1 do
      allThreads[c] :=
        TThread.CreateAnonymousThread(
          procedure
          var json :ISuperObject;
                rec:TExternalRecord;
          begin
            try
              json := SO(jsonText);
              try
                 rec := TJSON.Parse<TExternalRecord>(json);  // this raises random exceptions!
              except
                 on e:exception do begin
                   TInterlocked.Increment(errorCount);
                   WriteLog(e.ClassName + ': '+e.Message);
                 end;
              end;

            finally
               TInterlocked.Decrement(activeThreadsCount);
            end;
          end);

    for c := 0 to high(allThreads) do begin
       t := allThreads[c];
       TThread.NameThreadForDebugging('I am thread #'+c.ToString,t.ThreadID);
       t.Start;
    end;

    repeat
      lblNThreads.Caption := activeThreadsCount.ToString;
      lblErrors.Caption := errorCount.ToString;
      sleep (100);
      Application.ProcessMessages;
    until activeThreadsCount = 0;

    lblThreadCount.Caption := activeThreadsCount.ToString;
    lblErrors.Caption := errorCount.ToString;

  finally
    btnBoom.Enabled := true;
    Writelog('Finished!');
  end;

end;

end.

TheFormU.dfm

object TheForm: TTheForm
  Left = 0
  Top = 0
  Caption = 'TheForm'
  ClientHeight = 442
  ClientWidth = 484
  Color = clBtnFace
  Font.Charset = DEFAULT_CHARSET
  Font.Color = clWindowText
  Font.Height = -12
  Font.Name = 'Segoe UI'
  Font.Style = []
  TextHeight = 15
  object Panel_78E65794: TPanel
    Left = 0
    Top = 0
    Width = 484
    Height = 145
    Align = alTop
    TabOrder = 0
    object lblNThreads: TLabel
      Left = 24
      Top = 15
      Width = 182
      Height = 15
      Caption = 'Number of threads to be launched'
    end
    object Label_4F53C536: TLabel
      Left = 119
      Top = 81
      Width = 101
      Height = 15
      Alignment = taCenter
      Caption = 'Exception Counter:'
    end
    object lblThreadCount: TLabel
      Left = 226
      Top = 62
      Width = 15
      Height = 15
      Caption = '---'
    end
    object lblErrors: TLabel
      Left = 226
      Top = 80
      Width = 15
      Height = 15
      Caption = '---'
    end
    object Label_7E8DB5CF: TLabel
      Left = 24
      Top = 63
      Width = 198
      Height = 15
      Caption = 'Number of currently running threads:'
    end
    object ctrlNThreads: TEdit
      Left = 22
      Top = 33
      Width = 89
      Height = 24
      TabOrder = 0
      Text = '50'
    end
    object btnBoom: TButton
      Left = 22
      Top = 104
      Width = 217
      Height = 25
      Caption = 'BOOM!'
      TabOrder = 1
      OnClick = btnBoomClick
    end
  end
  object log: TMemo
    Left = 0
    Top = 145
    Width = 484
    Height = 297
    Align = alClient
    Font.Charset = DEFAULT_CHARSET
    Font.Color = clWindowText
    Font.Height = -12
    Font.Name = 'Consolas'
    Font.Style = []
    Lines.Strings = (
      'log')
    ParentFont = False
    ScrollBars = ssBoth
    TabOrder = 1
  end
end
csm101 commented 1 year ago

I resolved by doing exactly what @CaptainNemo07 was suggesting... Is this project still being maintained? Is anyone aware of some fork by someone who has taken over the burden of keeping it maintained?

Tavel commented 1 year ago

I resolved by doing exactly what @CaptainNemo07 was suggesting...

Can you please to publish fixed version on https://github.com/csm101 ? It'll save a lot of time for other developers.

csm101 commented 1 year ago

Sure, just did it: https://github.com/csm101/x-superobject

I did actually use a global critical section around all the entire x-superobject methods, not around each single RTTI call, maybe it is a bit overkill, but it surely solves the problem

Tavel commented 1 year ago

Sure, just did it: https://github.com/csm101/x-superobject

I did actually use a global critical section around all the entire x-superobject methods, not around each single RTTI call, maybe it is a bit overkill, but it surely solves the problem

Thank you very much for your great work! My test project now runs with no issue on your fixed version.