Skip to content
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

RTTI and MultiThreading #141

Open
vilgan opened this issue Mar 30, 2021 · 8 comments
Open

RTTI and MultiThreading #141

vilgan opened this issue Mar 30, 2021 · 8 comments

Comments

@vilgan
Copy link

vilgan commented Mar 30, 2021

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
Copy link
Contributor

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

@vilgan
Copy link
Author

vilgan commented Mar 30, 2021

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
Copy link

Tavel commented Jan 8, 2022

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
Copy link

csm101 commented Dec 13, 2022

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
Copy link

csm101 commented Dec 14, 2022

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
Copy link

Tavel commented Dec 15, 2022

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 pushed a commit to csm101/x-superobject that referenced this issue Dec 16, 2022
…g by applying corrections suggested by @CaptainNemo07.

(there were also some TRTTIContexts that were wrongly Freed in "except" sections instead of "finally" sections)
@csm101
Copy link

csm101 commented Dec 16, 2022

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
Copy link

Tavel commented Dec 19, 2022

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.

Repository owner deleted a comment from mkproject-admin Feb 21, 2024
Repository owner deleted a comment from amd119 Feb 23, 2024
Repository owner deleted a comment from sandipbhuyan Mar 20, 2024
Repository owner deleted a comment from sandipbhuyan Mar 20, 2024
Repository owner deleted a comment from sandipbhuyan Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants
@csm101 @vkrapotkin @Tavel @vilgan and others