Delphi static array thread safety only with critical section

Question

I have a Win32 Thread (no TThread) that runs alle the time and iterates over a static array. The mainthread can modify fields of the array. What is the best way to make this thread-safe without components like TThreadList (for a no-vcl application), only with Windows Critical Sections (TRTLCriticalSection)?

Code:

type
  T = record
    Idx: Integer;
    Str: string;
    Num: Real;
    Enabled: Boolean;
  end;

var
  A: Array[0..9] of T;
  Cnt: Integer;
  CS: TRTLCriticalSection;

procedure thread;
var
  I: Integer;
begin
  while True do
  begin
    for I := Low(A) to High(A) do
    begin
      if A[I].Enabled then
      begin
        //modify some fields from A[I]

        Inc(A[I].Idx);
        if A[I].Idx >= 10 then
        begin
          A[I].Enabled := False;
          InterlockedDecrement(Cnt);
        end;
      end;
    end;
    if Cnt = 0 then Sleep(1);
  end;
end;

procedure Add(...); //called only from mainthread

  function GetFreeField: Integer;
  begin
    for Result := Low(A) to High(A) do
      if not A[Result].Enabled then Exit;
    Result := -1;
  end;

var
  I: Integer;
begin
  I := GetFreeField;
  if I = -1 then Exit;

  //set fields A[I]

  A[I].Enabled := True;
  InterlockedIncrement(Cnt);
end;

At the beginning the array is initialized with enabled = false and cnt = 0.

Is the following modification sufficient?

procedure thread;
var
  I: Integer;
begin
  while True do
  begin
    for I := Low(A) to High(A) do
    begin
      EnterCriticalSection(CS);
      if A[I].Enabled then
      begin
        LeaveCriticalSection(CS);
        //modify some fields from A[I]

        Inc(A[I].Idx);
        if A[I].Idx >= 10 then
        begin
          EnterCriticalSection(CS);
          A[I].Enabled := False;
          LeaveCriticalSection(CS);

          InterlockedDecrement(Cnt);
        end;
      end
      else
        LeaveCriticalSection(CS);
    end;
    if Cnt = 0 then Sleep(1);
  end;
end;

procedure Add(...); //called only from mainthread
var
  I: Integer;
begin
  I := GetFreeField;
  if I = -1 then Exit;

  //set fields A[I]

  EnterCriticalSection(CS);
  A[I].Enabled := True;
  LeaveCriticalSection(CS);

  InterlockedIncrement(Cnt);
end;

`TThreadList` is not a part of the `VCL`, its in System.Classes. Adding/removing items in such a list is thread-safe. Manipulating list items has to be protected though. — LU RD, Oct 10 '12 at 11:56

David Heffernan · Accepted Answer · 2012-10-10T12:14:25.077

1

It looks to me as though your design is that:

The main thread only ever switches the Enabled flag from False to True.
The worker thread only ever switches the flag in the opposite direction.
No code other than what we see here accesses the array.

If that is true, the original code without the critical section is already thread safe. At least it is on hardware that uses a strong memory model. For example the Intel x86 or x64 architectures. The Enabled boolean acts as a synchronisation barrier between the threads.

However, your entire design looks flawed to me. The while True loop and the Sleep causes me some alarm. That thread is going run repeatedly for no good reason. Surely you should only be executing the code in the thread when the main thread has made modifications to the array. I'd prefer the use of a signal (for example a Windows event) to wake up the thread.

edited Oct 10 '12 at 12:14

answered Oct 10 '12 at 12:02

David Heffernan

601,492
42
1,072
1,490

Read/Write a Byte (Boolean) is a guarantied atomic operation on Intel processors; strong memory ordering is not supported by modern processors, it is a different concept and not related to the original question. – kludg Oct 10 '12 at 12:17
@Serg If code like this ran under a weak memory model then the boolean writes could be re-ordered before the mods to A[i] fields. Of course, it won't run under a weak memory model. And x86 and x64 have strong memory models. – David Heffernan Oct 10 '12 at 12:30
The original code is incomplete and unclear, I can't see what exactly should be protected, so hard to say anything definite about its thread safety. – kludg Oct 10 '12 at 12:41
@Serg We have to assume, as I did in point 3 in the answer, that there aren't other bits of code accessing `A`. With that assumption, it's pretty easy to analyse. – David Heffernan Oct 10 '12 at 12:50
If so there is nothing to protect and no problems with thread safety at all, even if `A[I].Enabled := True/False` is not an atomic operation, but the code 'as is' makes no sense to me. – kludg Oct 10 '12 at 12:59

Delphi static array thread safety only with critical section

1 Answers1