Search code examples
delphidelphi-11-alexandria

Multi-threaded writing on an Array of Strings should be protected with a critical section?


I'm looking after a very occasional AV in our RAD Server, and I'm wondering if the problem could be in an unguarded multi-threaded writing of an array of strings.

It's something similar to this:

function GetQuerys(SQLs: TStringDynArray): TStringDynArray;
begin
  var JSONs: TStringDynArray;
  SetLength(JSONs, Length(SQLs));

  TParallel.For(0, Length(SQLs) - 1,
    procedure(i: integer)
    begin
      var Query := TFDQuery.Create(nil);
      try
        var Cn := TFDConnection.Create(Query);  // We use a different Connection for every Query
                                                // Each Connection will be released when its Query is released, through the Ownership

        Cn.ConnectionDefName := 'MyPooledConnection';   // The Connections are Pooled=True to avoid connection delays
        Query.Connection := Cn;
        Query.SQL.Text := SQLs[i];

        Query.Open;

        var Stream := TStringStream.Create;
        try
          Query.SaveToStream(Stream, sfJSON);
          JSONs[i] := Stream.DataString;        // We return the JSON Data of every Query
        finally
          Stream.Free;
        end;
      finally
        Query.Free;
      end;
    end
  );
  Result := JSONs;
end;

I know that you have to protect any multi-threaded modification access to the same object using critical sections, but is it also necessary to protect with critical sections the writing of the JSONs array ?.

I thought that when you don't change the size of the array items then it's safe to change them unguarded. And an array of strings is just an array of pointers to the real strings that are stored in the memory pool, isn't it ?.

So, is it really necessary to replace the JSONs[i] := Stream.DataString; with:

WritingLock.Acquire;
try
  JSONs[i] := Stream.DataString;
finally
  WritingLock.Release;
end;

... or would that be over-programming ?.

Thank you.


Solution

  • I have located the problem. It's in these lines:

    var Cn := TFDConnection.Create(Query);
    Cn.ConnectionDefName := 'MyPooledConnection'; 
    

    If I guard them within a critical section then everything runs fine.

    class var
      FConnectionLock: TCriticalSection;
    ...
    ...
    ...
      FConnectionLock.Acquire;           
      try                                
        var Cn := TFDConnection.Create(Query);
        Cn.ConnectionDefName := 'MyPooledConnection';
      finally
        FConnectionLock.Release;
      end;
    ...
    ...
    ...
    initialization
      TMyResource.FConnectionLock := TCriticalSection.Create;
    
    finalization
      TMyResource.FConnectionLock.Free;
    

    My guess is that although every Cn: TFDConnection are separate objects, they are Pooled=True, so they all race to access to the same shared pool of connections to acquire one.

    Guarding within a critical section this acquisition of a connection from the pool solves my sporadic AVs.