2

I have a TThread object named TTestThread. When the threads are created, they make a local copy of the global configuration variables. The user is allowed to change the global configuration variables at any time, and when he does so, all threads are notified of the change via their local UpdateLocalCopyOfConfigVariables variable. The threads don't rely on the global configuration variables directly because they can be modified at any point by the user, which would create a race condition if the threads were to access them at the same time.

Here's TTestThread:

type
  TTestThread = class(TThread)
  private
    LocalConfigA : String;
    LocalConfigB : Integer;
    procedure UpdateLocalConfigIfNecessary;
  protected
    procedure Execute; override;
  public
    UpdateLocalCopyOfConfigVariables : Boolean;
    constructor Create;
  end;

implementation

constructor TTestThread.Create;
begin
  inherited Create(false);
  UpdateLocalCopyOfConfigVariables := true;
end;

procedure TTestThread.UpdateLocalConfigIfNecessary;
begin
  WaitForSingleObject(ConfigurationLocker, INFINITE);
  if (UpdateLocalCopyOfConfigVariables) then
  begin
    LocalConfigA := GlobalConfigA;
    LocalConfigB := GlobalConfigB;
    UpdateLocalCopyOfConfigVariables := false;
  end;
  ReleaseMutex(ConfigurationLocker);
end;

procedure TTestThread.Execute;
begin
  while (not(Terminated)) do
  begin
    UpdateLocalConfigIfNecessary;
    // Do stuff
  end; 
end;

As you can see, I have a mutex in place to avoid the sort of race condition described earlier. WaitForSingleObject(ConfigurationLocker, INFINITE); is called when the user changes the global configuration variables:

procedure ChangeGlobalConfigVariables(const NewGlobalConfigA : String ; NewGlobalConfigB : Integer);
var
  I : Integer;
begin
  WaitForSingleObject(ConfigurationLocker, INFINITE);

  GlobalConfigA := NewGlobalConfigA;
  GlobalConfigB := NewGlobalConfigB;

  for I := 0 to ThreadList.Count - 1 do
    TTestThread(ThreadList[I]).UpdateLocalCopyOfConfigVariables := true;

  ReleaseMutex(ConfigurationLocker);
end;

The thing is, while it prevents the threads from updating their local copy of configuration variables at the same time the global configuration variables are being changed, it also prevents two threads from updating their local configuration at the same time – even if the global configuration variables are not being changed. As far as I know, race condition is an issue when there's writing going on. If the global configuration variables are the same, then the threads can all update their local copy at the same time with no issues.

Am I right? If so, is there a way to fix this issue? Granted, it's not a big one, but I still feel like there has to be a better solution...

whosrdaddy
  • 11,720
  • 4
  • 50
  • 99
Pascal Bergeron
  • 761
  • 3
  • 12
  • 27

2 Answers2

5

It seems a good point for TMultiReadExclusiveWriteSynchronizer work

MBo
  • 77,366
  • 5
  • 53
  • 86
  • Just be careful with `TMREWSync`, it does have some known issues in some cases. If you are running on Windows Vista+, consider Win32 [Slim Reader/Writer (SRW) Locks](https://msdn.microsoft.com/en-us/library/windows/desktop/aa904937.aspx) as an alternative. – Remy Lebeau Nov 10 '15 at 17:11
  • What kind of issues are we talking about? I made a quick search, but I'm not sure what you are referring to exactly. – Pascal Bergeron Nov 10 '15 at 17:56
4

There is no way to lock code only when there is writing access. MBo's answer shows a way how to allow multiple threads reading a variable while writing is locked.

However, reading the global configuration object should be really fast. So that can't be the bottleneck here.

How to process this faster?

1. Use TCriticalSection instead of a mutex

A critical section is much faster than a mutex. A mutex can be used for multiple processes while a critical section works in the current process only. You don't need this expensive kind of locking. Exchange ConfigurationLocker by a variable like this ConfigurationCriticalSection: TCrticalSection. You need to create the object on startup since it is used as a global variable.

You use a critical section in a similar way. Example:

procedure TTestThread.UpdateLocalConfigIfNecessary;
begin
  ConfigurationCriticalSection.Enter;
  try 
    if (UpdateLocalCopyOfConfigVariables) then
    begin
      LocalConfigA := GlobalConfigA;
      LocalConfigB := GlobalConfigB;
      UpdateLocalCopyOfConfigVariables := false;
    end;
  finally
    ConfigurationCriticalSection.Leave;
  end;
end;

The try..finally..end pattern is very important here. If you don't use it you can end up in a dead lock when an exception has been raised between Enter and Leave. You need to apply the same pattern when you use a mutex with WaitForSingleObject and ReleaseMutex.

2. Send copies of the new values to the threads

Don't let the threads access the global config. There are many different patterns out there to inform other objects about changes. One way would be to call a method that provides the values. This could look like this:

TTestThread(ThreadList[I]).UpdateLocalCopyOfConfigVariables(GlobalConfigA, GlobalConfigB);

So the call to TTestThread(ThreadList[I]).UpdateLocalCopyOfConfigVariables := true would be replaced.

The thread objects would store the new values and apply them on specific situations.

Community
  • 1
  • 1
Wosi
  • 41,986
  • 17
  • 75
  • 82
  • You are right when saying there is no real, massive bottleneck. It just felt "odd" to me to have threads waiting for each other when they could actually read the global configuration variables at the same time with no issues. I appreciate your answer though! – Pascal Bergeron Nov 10 '15 at 16:29
  • A reader/writer lock would be more appropriate than a mutex or critical section. Such a lock would allow multiple threads to read from the config at the same time without waiting on each other, while still providing an exclusive lock when you need to make changes to the config. – Remy Lebeau Nov 10 '15 at 17:13