6

I'm having problems with multi thread in delphi. I Have a list of names (something about 2.000 names), and I need to get some data of each name in my site. My system works perfectly, except the thread control.

I want to create 10 threads, and, when some thread terminate, create another... until end of list.

var
 Form1: TForm;
 tCount: Integer;  //threads count

implementation

type
 TCheck = class(TThread)
 public
  constructor Create(Name: string);
  destructor Destroy; Override;
 protected
  procedure Execute; Override;
 end;

 MainT = class(TThread)
 protected
  procedure Execute; Override;
 end;

destructor TCheck.Destroy;
begin
 Dec(tCount);
end;

procedure MainT.Execute;
var
 i: Integer;
 Load: TStringList;
begin
 Load:=TStringList.Create;
 Load.LoadFromFile('C:\mynames.txt');

 for i:= 0 to Load.Count -1 do
 begin

  if tCount = 10 then  //if we have 10 threads running...
  begin
   repeat
    Sleep(1);
   until tCount < 10;
  end;

  TCheck.Create(Load.Strings[i]);
  TCheck.Start;
  Inc(tCount);

 end;

end;  // end of procedure

Well, I didn't put the TCheck.Constructor because the problem is the method how I'm check the number of created threads. I mean, my software just stop, without any error message, sometimes check 500 names, sometimes 150 names...

Sorry for Bad English.

Kiritonito
  • 414
  • 1
  • 9
  • 19
  • Could you post TCheck ctor? – Martin James Mar 25 '13 at 18:39
  • 5
    Also, if you want 10 threads, make 10 threads and have them process all your work by queueing stuff to them. Don't continually create/terminate/destroy them. Forget tCount and micro-managing threads. – Martin James Mar 25 '13 at 18:42
  • 2
    Yeah. You need a producer that fills a threadsafe queue, and 10 consumers that drain it. – David Heffernan Mar 25 '13 at 19:31
  • Here is a nice threadsafe queue: TThreadStringList http://www.swissdelphicenter.ch/torry/showcode.php?id=2167 – Chris Thornton Mar 25 '13 at 19:36
  • @Chris No, here's is what a threadsafe queue looks like: http://stackoverflow.com/questions/15027726/how-to-make-a-thread-finish-its-work-before-being-freed/15033839#15033839 – David Heffernan Mar 25 '13 at 19:55
  • Why not use containers from OmniThreadLibrary contained in OmniCommon. It can be used as a standalone unit with lots of good code. Blocking queue, lock free queue, all highest quality code. I also have a good and fast implementation of the thread safe queue with micro locking but still, OTL beats all. – Runner Mar 25 '13 at 21:12
  • 2
    @Chris Thornton, that example of thread safe TStringList is just horrible :) – Runner Mar 25 '13 at 21:12
  • Yes @Chris, it made my eyes bleed – David Heffernan Mar 25 '13 at 21:15
  • @Runner my eyes not much better either. CBA to respond at the time to a class with overridden methods with locks in them all. – Martin James Mar 26 '13 at 00:55
  • Is it my mistake or anybody does not talk about unacceptable usage of external variable for thread (tCount) without TCriticalSection? @user2208678 You must read more docu about thread programming. As alternative use OmniThreadLibrary (see examples for it) – Abelisto Mar 26 '13 at 01:20
  • 1
    @Abelisto 'Forget tCount and micro-managing threads' - this encompasses the fact that tCount is not thread-safe. The best way to make tCount thread-safe is to not use it at all. – Martin James Mar 26 '13 at 09:40

2 Answers2

3

Here is a threadsafe queue solution using generics.

Define how many consumer threads you want, the queue depth and just run the DoSomeJob procedure from a thread.

Define your job working with a string as a generic procedure (in CaptureJob).

When the queue is empty, the consumer threads will be destroyed. The DoSomeJob procedure waits until all jobs are ready. You can easily turn this into a generic worker pool, reusing the threads without destroying them. The generic structure of the job items also make them suitable to handle different kinds of work.

Note this queue works on XE2 and above. If you are on a older delphi version, look for a similar threadsafe queue as suggested in comments.

uses
  Classes,SyncObjs,Generics.Collections;

Type
  TMyConsumerItem = class(TThread)
  private
    FQueue : TThreadedQueue<TProc>;
    FSignal : TCountDownEvent;
  protected
    procedure Execute; override;
  public
    constructor Create( aQueue : TThreadedQueue<TProc>; aSignal : TCountdownEvent);
  end;

constructor TMyConsumerItem.Create(aQueue: TThreadedQueue<TProc>);
begin
  Inherited Create(false);
  Self.FreeOnTerminate := true;
  FQueue := aQueue;
  FSignal := aSignal;
end;

procedure TMyConsumerItem.Execute;
var
  aProc : TProc;
begin
  try
    repeat
      FQueue.PopItem(aProc);
      if not Assigned(aProc) then
        break; // Drop this thread
      aProc();
    until Terminated;
  finally
    FSignal.Signal;
  end;
end;

procedure DoSomeJob(myListItems : TStringList);
const
  cThreadCount = 10;
  cMyQueueDepth = 100;
var
  i : Integer;
  aQueue : TThreadedQueue<TProc>;
  aCounter : TCountDownEvent;
  function CaptureJob( const aString : string) : TProc;
  begin
    Result :=
      procedure
      begin
        // Do some job with aString
      end;
  end;
begin
  aQueue := TThreadedQueue<TProc>.Create(cMyQueueDepth);
  aCounter := TCountDownEvent.Create(cThreadCount);
  try
    for i := 1 to cThreadCount do
      TMyConsumerItem.Create(aQueue,aCounter);
    for i := 0 to myListItems.Count-1 do begin
      aQueue.PushItem( CaptureJob( myListItems[i]));
    end;
  finally
    for i := 1 to cThreadCount do
      aQueue.PushItem(nil);
    aCounter.WaitFor;  // Wait for threads to finish
    aCounter.Free;
    aQueue.Free;
  end;
end;

NB: Ken explains why your initialization and start of threads are wrong. This proposal shows a better structure to handle this type of problems in a more generic way.

LU RD
  • 34,438
  • 5
  • 88
  • 296
  • This doesn't explain how the question's code doesn't work. There's also no indication in the original question of what version of Delphi is being used, so it's not clear generics are an option. (See the last two paragraphs of my answer above.) :-) (Not downvoting - just leaving a comment.) – Ken White Mar 25 '13 at 22:42
  • 1
    @KenWhite, sorry, this answer was just to show how this problem could be solved with a better structure. All credits to you for spotting the error in posters code. – LU RD Mar 25 '13 at 22:59
1

If you don't declare a variable to hold the return value of TCheck.Create, you can't access TCheck.Start (there's no instance of TCheck you can use to access the Start method).

The proper way would be to declare a var Check: TCheck; inside MainT.Execute, and then store the value returned:

Check := TCheck.Create(Load[i]);  { See note below }
Check.Start;
Inc(tCount);

NOTE The default property of TStringList is Strings, so you don't need to use it. You can just access Strings directly as I have above. The next two lines are exactly the same thing (but obviously one is shorter and easier to type):

Load.Strings[i];
Load[i];

If you don't want to keep a reference to TCheck, just change your code to be a with block (including the begin..end, and containing no other code in the block (this is the only way I ever recommend using a with):

with TCheck.Create(Load[i]) do
begin
  Start;
  Inc(tCount);
end;

With that being said, there are much better ways you can do this instead of creating/destroying all kinds of threads. As others have said, you could have a list of 10 threads and queue the work for them, so that each would process an item from Load and then return to get another item to process when done, and repeat until the list is complete. It's hard to say exactly how you would do that, because that would depend on your Delphi version. (There are libraries available that will do most of the work for you, like OMNIThreadLibrary, but it's not available for some older versions of Delphi. Recent versions of Delphi also support TQueue and TObjectQueue and some other types and functionality that might be very helpful.

(If you have a different question about how to do this in a queue with a limited number of threads, that should be a new question, not something you add to this one.)

Ken White
  • 123,280
  • 14
  • 225
  • 444