Synchronize multiple threads

Question

I'm trying to make a multithreaded application (up to lets say 100 thread) in Delphi XE5, all the thread are going to use/change a listbox in the main form, here is my code: Main Unit:

private
  //list to hold the dynamic created threads
  SendThreads : TObjectList;
public
  mutex : boolean;
...
procedure TMainForm.FormCreate(Sender: TObject);
begin
   ...
   mutex := false;
   SendThreads := TObjectList.Create;
end;

...

//Button to create and start the threads
procedure TMainForm.btnWorkClick(Sender: TObject);
var
  Thread : TSendThread;
  i, nbr : integer;
begin
  if SendThreads.Count < spnThreadCount.Value then
  begin
    for I := 1 to spnThreadCount.Value - SendThreads.Count do
    begin

      nbr := SendThreads.Add(TSendThread.Create(true));

      Thread := TSendThread(SendThreads.Items[nbr]);

      Thread.FreeOnTerminate := true;
      Thread.Start;
    end;
  end;
end;

Thread Unit:

    uses MainUnit;

    procedure TSendThread.Execute;
    begin
      QueryList;
    end;

//Basically, this procedure checks if the item in the listbox contains '- Done.' which means that this
//item has been done by another thread, if not, the current thread deal with this item.
procedure TSendThread.QueryList;
var i : integer;
    S : String;
begin
  i := 0;
  while i < MainForm.lstURL.Count do
  begin

    while MainForm.mutex do;

    MainForm.mutex := true;

    if pos(' - Done.', MainForm.lstURL.Items[i]) = 0 then
    begin
      S := MainForm.lstURL.Items[i];
      Delete(S, 1, pos('?txt=', S) + length('?txt=') - 1);
      MainForm.Memo1.Lines.Add(MainForm.lstURL.Items[i]);
      MainForm.lstURL.Items[i] := MainForm.lstURL.Items[i] + ' - Done.';

      MainForm.mutex := false;
      SendAd(URL, S);
    end
    else
    begin
      Inc(i);
      MainForm.mutex := false;
    end;
  end;
end;

This method works if the number of the threads are less than 4, but if it's more i get redundant result (2 or more threads do the same item). Now i'm fairly new to threads and multithreading, and i was wondering if this is the right way to do it.

Answer 1

In addition to what Ken said about UI safety, you are getting threads processing the same item because you have a race condition on your mutex variable. Multiple threads may see mutex=false at the same time as they are not actually syncing with each otber. You need to use a true mutex. Look at the TMutex class. Or just wrap the majority of your code in TThread.Synchronize() and let the VCL handle the syncing for you. But then that defeats the purpose of using threads.

You are using the completely wrong design for your requirement. You need to separate your worker thread logic from your UI logic. There are a few different ways you can do that.

1. Put your work jobs into a thread-safe queue, like TThreadList<T> or TThreadedQueue<T>. Each thread can check the queue periodically and pull the next available job if there is one.

   A. A variation is to use an I/O Completion Port as the queue. Post jobs to the IOCP using PostQueuedCompletionStatus(), and have each thread use GetQueuedCompletionResult() to receive jobs. This allows the OS to do all the queuing and pulling for you, while allowing threads to sleep when there are no jobs available.

2. Put your threads into a pool in a sleeping state. When a new job is ready, check the pool. If a thread is available, pull it from the pool, pass the job to it, and wake it up. Otherwise put the job in a thread-safe queue. When a job is done, have that thread check the queue. If a job is available, pull it from the queue, otherwise put the thread in the poll and put it back to sleep.

In any case, when a job is done being processed, that thread can notify the main thread of the job result, using TThread.Synchronize(), TThread.Queue(), or any other inter-thread communication of your choosing. The main thread can then update the UI as needed.

At no point should the threads touch the UI to discover new jobs.