Search code examples
windowsdelphiwinapidelphi-7

How to wait for multiple events?


There are few threads in a Application in which I am working with. Those threads are setup to FreeOnTerminate and I am not allowed to change this behavior. I saw something weird in the way the old developer was awaiting for some signals in the main thread, As Followed:

Let:

var FWaits: array of TEvent;
var FThreads: array of TBkgThread;
const C = 10;

for Each Thread, we have an Event, then Length(Threads) = Length(FWaits)

  for I:= 0 to C-1 do
  begin
    FWaits[I]:= TSimpleEvent.Create;
    FThreads[I]:= TBkgThread.Create(FWaits[I]); //The handle of the Event
  end;

  [CODE]
  for I:= 0 to Length(FWaits)-1 do
    case FWaits[I].WaitFor(INFINITE) of
      wrError: begin 
                 NotifyError; //Code Irrevelant; 
                 Break;
               end;
      wrSignaled: TryNotifyUser(I); //Code Irrevelant; 
      wrAbandoned: TryNotifyAbandon(I); //Code Irrevelant; 
      wrTimeout: TryNotifyTimeOut(I); //Code Irrevelant; 
    end;

This is programmed in the worker thread:

destructor TBkgThread.Destroy;
begin
  inherited;
  FEvent.SetEvent;
end;

I don't know if signaling the event after calling inherited is ok in this case, but this is not the problematic part.

I know of WaitForMultipleObjects, so I tried to reduce the code marked with [CODE] to this:

var Handles: array of THandle;

  SetLength(Handles, Length(FWaits));
  for I:= 0 to Length(FWaits)-1 do
    Handles[I]:= FWaits[I].Handle;

  case WaitForMultipleObjects(Length(Handles), @Handles[0], TRUE, INFINITE) of
    WAIT_FAILED: begin Label1.Caption:= 'WAIT_FAILED'; RaiseLastWin32Error; end;
    WAIT_OBJECT_0: Label1.Caption:= 'WAIT_OBJECT_0';
    WAIT_ABANDONED_0: Label1.Caption:= 'WAIT_ABANDONED_0';
    WAIT_TIMEOUT: Label1.Caption:= 'WAIT_TIMEOUT';
  end;

But it is raising an windows error: code 6.

How can I correctly wait for multiple events?

[UPDATE]

TBkgThread = class(TThread)
  private
    FEvent: TEvent;
  protected
    procedure Execute; override;
  public
    constructor Create(AEvent: TEvent);
    destructor Destroy; override;
  end;

  TForm1 = class(TForm)
    edThreads: TLabeledEdit;
    Button1: TButton;
    Button2: TButton;
    Label1: TLabel;
    procedure Button1Click(Sender: TObject);
    procedure Button2Click(Sender: TObject);
  private
    FThreads: array of TBkgThread;
    FWaits: array of TEvent;
  public
    { Public declarations }
  end;

{ TBkgThread }

constructor TBkgThread.Create(AEvent: TEvent);
begin
  inherited Create(False);
  FreeOnTerminate:= True;
  FEvent:= AEvent;
end;

destructor TBkgThread.Destroy;
begin
  inherited;
  FEvent.SetEvent;
end;

procedure TBkgThread.Execute;
var I,J,K: Integer;
begin
  while not Terminated do
  begin
    for I:= 0 to 10000 div 20 do
      for J:= 0 to 10000 div 20 do
        for K:= 0 to 10000 div 20 do;
    Sleep(1000);
  end;
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  I, C: Integer;
begin
  C:= StrToIntDef(edThreads.Text, 0);
  if C > 0 then
  begin
    SetLength(FThreads, C);
    SetLength(FWaits, C);
    for I:= 0 to C-1 do
    begin
      FWaits[I]:= TSimpleEvent.Create();
      FThreads[I]:= TBkgThread.Create(FWaits[I]);
    end;
  end;
end;

procedure TForm1.Button2Click(Sender: TObject);
var I: Integer;
    Handles: array of THandle;
begin
  for I:= 0 to Length(FWaits)-1 do
    FThreads[I].Terminate;

  SetLength(Handles, Length(FWaits));
  for I:= 0 to Length(FWaits)-1 do
    Handles[I]:= FWaits[I].Handle;

  case WaitForMultipleObjects(Length(Handles), @Handles[0], TRUE, INFINITE) of
    WAIT_FAILED: begin RaiseLastWin32Error; Label1.Caption:= 'WAIT_FAILED'; end;
    WAIT_OBJECT_0: Label1.Caption:= 'WAIT_OBJECT_0';
    WAIT_ABANDONED_0: Label1.Caption:= 'WAIT_ABANDONED_0';
    WAIT_TIMEOUT: Label1.Caption:= 'WAIT_TIMEOUT';
  end;

//  for I:= 0 to Length(FWaits)-1 do
//    case FWaits[I].WaitFor(INFINITE) of
//      wrError: begin Label1.Caption:= 'WAIT_FAILED'; Break; end;
//      wrSignaled: Label1.Caption:= 'WAIT_OBJECT_0';
//      wrAbandoned: Label1.Caption:= 'WAIT_ABANDONED_0';
//      wrTimeout: Label1.Caption:= 'WAIT_TIMEOUT';
//    end;
end;

Solution

  • You pass the address of the second wait handle instead of the first. Replace

    @Handles[1]
    

    with

    @Handles[0]
    

    Some other suggestions:

    1. You have to call RaiseLastWin32Error (or indeed RaiseLastOSError) immediately so that GetLastError can retrieve the error code for the desired API call. You call it after setting a label caption, something that will very likely result in an incidental call to SetLastError.
    2. It is better to set the event in an overridden DoTerminate method. That way you don't call methods on FEvent from the destructor. If the constructor raises, then FEvent can be nil.
    3. You should not retain references to free-on-terminate threads. Since the thread object can be disposed at any time, you can readily end up holding a reference to a destroyed object.

    That final point is very important. You clearly don't adhere to that point when you loop around all the threads calling Terminate on them. You need to decide whether to use free-on-terminate, or whether you want to retain references to the threads. You have to pick one or other option. You cannot have it both ways.

    If you wish to use free-on-terminate threads, then you would need to use a separate event to signal cancellation. Pass that event to the thread on construction. Instead of testing for Terminated in the thread method, test for that event being set. Set the event when you wish to cancel.

    However, all of this is very strange. You say that you wish to use free-on-terminate threads, but then you also want to wait for all the threads to complete. The use of free-on-terminate threads then forces you to create extra events to deal with the waiting and the cancellation. Because you cannot retain references to the thread. Now, you've got to free the events that you created. So, you've avoided having to free the threads, but replaced that with a requirement to free the events.

    Your approach is much more complex than it needs to be. In my view you should:

    • Stop using free-on-terminate threads.
    • Remove all the events.
    • Wait on the thread handles.
    • Free the threads when the wait completes.

    Only use free-on-terminate threads when you don't need to wait for the thread.