Does TCriticalSection allow multithreaded access to a variable?

Question

I want to syncronize threads to access a common variable. Imagine that I have N threads and each of them can acess a global instance of a variable of type TSyncThreds.

Can I call the methods IncCount, DecCount? Or i will have problem with concurrent threads accessing a same instance of a object?

I just syncronize the access to the FCcounter Variable...

type
  TSyncThreads = class

  public
    FCounterGuard: TCriticalSection;
    FCounter: integer;
    FSimpleEvent: TSimpleEvent;

    constructor Create();
    procedure Wait();
    procedure IncCounter();
    procedure DecCounter();
  end;


var
  SyncThread: TSyncThreads;

implementation
uses
  Math, Windows, Dialogs;



{ TSyncThreads }

procedure TSyncThreads.DecCounter;
begin
  FCounterGuard.Acquire;
  Dec(FCounter);
  if FCounter = 0 then
    FSimpleEvent.SetEvent;
  FCounterGuard.Release;
end;

procedure TSyncThreads.IncCounter;
begin
  FCounterGuard.Acquire;
  Inc(FCounter);
  FCounterGuard.Release;
end;

constructor TSyncThreads.Create();
begin
  FCounter := 0;
  FSimpleEvent := TSimpleEvent.Create;
  FCounterGuard := TCriticalSection.Create;
  FSimpleEvent.ResetEvent;
end;

procedure TSyncThreads.Wait;
var
  ret: TWaitResult;
begin
  ret := FSimpleEvent.WaitFor(INFINITE);
end;

Answer 1

Yes, your code is fine, if a bit overkill for the task at hand. You're worried about multiple threads accessing the same object, but that's exactly what synchronization objects like TCriticalSection and TEvent are designed for. Imagine how pointless such classes would be if they couldn't be accessed concurrently.

You don't really need a critical section to protect access to your counter. You can use InterlockedIncrement and InterlockedDecrement for more lightweight access. They return the variable's previous value, so if InterlockedDecrement returns 1, then you know it's time to set your event.

If there's an upper bound on the counter, then you might even be able to avoid all of this code and use a semaphore instead. Or you can give each thread its own event to set, and then use WaitForMultipleObjects to wait on all the events; it will return once all the threads have set their events, so you don't have to synchronize access to any shared variable.