Reputation: 2461
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;
Upvotes: 2
Views: 1492
Reputation: 612993
You pass the address of the second wait handle instead of the first. Replace
@Handles[1]
with
@Handles[0]
Some other suggestions:
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
.DoTerminate
method. That way you don't call methods on FEvent
from the destructor. If the constructor raises, then FEvent
can be nil
.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:
Only use free-on-terminate threads when you don't need to wait for the thread.
Upvotes: 3