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;
It looks to me as though your design is that:
Enabled
flag fromFalse
toTrue
.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 theSleep
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.