13

What is the best way to write a Delphi DUnit test for a TThread descendant when FreeOnTerminate = True? The TThread descendant returns a reference which I need to test for, but I can't figure out how to wait for the thread to finish in the test...

unit uThreadTests;

interface

uses
  Classes, TestFramework;

type

  TMyThread = class(TThread)
  strict private
    FId: Integer;
  protected
    procedure Execute; override;
  public
    constructor Create(AId: Integer);
    property Id: Integer read FId;
  end;

  TestTMyThread = class(TTestCase)
  strict private
    FMyId: Integer;
    procedure OnThreadTerminate(Sender: TObject);
  protected
    procedure SetUp; override;
    procedure TearDown; override;
  published
    procedure TestMyThread;
  end;

implementation

{ TMyThread }

constructor TMyThread.Create(AId: Integer);
begin
  FreeOnTerminate := True;
  FId := AId;

  inherited Create(False);
end;

procedure TMyThread.Execute;
begin
  inherited;

  FId := FId + 1;
end;

{ TestTMyThread }

procedure TestTMyThread.TestMyThread;
//var
//  LThread: TMyThread;
begin
//  LThread := TMyThread.Create(1);
//  LThread.OnTerminate := OnThreadTerminate;
//  LThread.WaitFor;
//  CheckEquals(2, FMyId);
//  LThread.Free;
///// The above commented out code is only useful of FreeOnTerminate = False;

  with TMyThread.Create(1) do
  begin
    OnTerminate := OnThreadTerminate;
    WaitFor; /// Not sure how else to wait for the thread to finish?
  end;

  CheckEquals(2, FMyId);
end;

procedure TestTMyThread.OnThreadTerminate(Sender: TObject);
begin
  FMyId := (Sender as TMyThread).Id;
end;  /// When FreeOnTerminate = True - THIS LINE CAUSES ERROR: Thread Error the handle is invalid

procedure TestTMyThread.SetUp;
begin
  inherited;

end;

procedure TestTMyThread.TearDown;
begin
  inherited;

end;

initialization
  RegisterTests([TestTMyThread.Suite]);


end.

Any ideas would be welcomed.

Delphi 2010.

Rick Wheeler
  • 1,142
  • 10
  • 22

4 Answers4

4

Subclass the thread to make it more testable. TThread and TObject provide enough hooks that you can add sensing variables to observe that it reaches certain points with the states you want it to have.

I see three aspects to this particular class that you might wish to test:

  1. It computes a value for its Id property based on the value sent to the constructor.
  2. It computes the new Id property in the new thread, not the thread that calls the constructor.
  3. It frees itself when it's finished.

All those things are testable from a subclass, but hard to test otherwise without making changes to the thread's interface. (All the other answers so far require changing the thread's interface, such as by adding more constructor arguments or by changing the way it starts itself. That can make the thread harder, or at least more cumbersome, to use in the real program.)

type
  PTestData = ^TTestData;
  TTestData = record
    Event: TEvent;
    OriginalId: Integer;
    FinalId: Integer;
  end;

  TTestableMyThread = class(TMyThread)
  private
    FData: PTestData;
  public
    constructor Create(AId: Integer; AData: PTestData);
    destructor Destroy; override;
    procedure AfterConstruction; override;
  end;

constructor TTestableMyThread.Create(AId: Integer; const AData: PTestData);
begin
  inherited Create(AId);
  FData := AData;
end;

destructor TestableMyThread.Destroy;
begin
  inherited;
  FData.FinalId := Id;
  // Tell the test that the thread has been freed
  FData.Event.SetEvent;
end;

procedure TTestableMyThread.AfterConstruction;
begin
  FData.OriginalId := Id;
  inherited; // Call this last because this is where the thread starts running
end;

Using that subclass, it's possible to write a test that checks the three qualities identified earlier:

procedure TestTMyThread.TestMyThread;
var
  Data: TTestData;
  WaitResult: TWaitResult;
begin
  Data.OriginalId := -1;
  Data.FinalId := -1;
  Data.Event := TSimpleEvent.Create;
  try
    TTestableMyThread.Create(1, @Data);

    // We don't free the thread, and the event is only set in the destructor,
    // so if the event is signaled, it means the thread freed itself: That
    // aspect of the test implicitly passes. We don't want to wait forever,
    // though, so we fail the test if we have to wait too long. Either the
    // Execute method is taking too long to do its computations, or the thread
    // isn't freeing itself.
    // Adjust the timeout based on expected performance of Execute.
    WaitResult := Data.Event.WaitFor(5000);
    case WaitResult of
      wrSignaled: ; // This is the expected result
      wrTimeOut: Fail('Timed out waiting for thread');
      wrAbandoned: Fail('Event was abandoned');
      wrError: RaiseLastOSError(Data.Event.LastError);
      else Fail('Unanticipated error waiting for thread');
    end;

    CheckNotEquals(2, Data.OriginalId,
      'Didn''t wait till Execute to calculate Id');
    CheckEquals(2, Data.FinalId,
      'Calculated wrong Id value');
  finally
    Data.Event.Free;
  end;
end;
Rob Kennedy
  • 161,384
  • 21
  • 275
  • 467
  • Thanks for the answer Rob. I have not had time to test solution yet will try tomorrow. – Rick Wheeler Mar 21 '13 at 10:29
  • Hi Rob, thanks your answer seems to be working great. I've accepted this as the answer. Also, there was an interesting answer posted below using anonymous threads. I'm still on Delphi 2010 so I don't have access to it, but looks like an elegant solution. Will test that as well once I've upgraded. Cheers. – Rick Wheeler Mar 26 '13 at 05:17
2

Create the thread in a suspended state, then set the OnTerminate and finally Resume the thread.

In your test class, define a private boolean field FThreadDone which is initialized with false and set to true by the OnTerminate Eventhandler.

Also, your constructor logic is a bit dirty, as you should not initialize field prior to calling the inherited constructor.

So:

constructor TMyThread.Create(AId: Integer);
begin
  inherited Create(true);
  FreeOnTerminate := True;
  FId := AId;
end;
...
procedure TestTMyThread.TestMyThread;
begin
  FThreadDone := False;
  with TMyThread.Create(1) do begin // Note: Thread is suspended...
    OnTerminate := OnThreadTerminate;
    // Resume;                         // ... and finally started here!
    Start;

  end;
  While not FThreadDone do Application.ProcessMessages;
  CheckEquals(2, FMyId);
end;

procedure TestTMyThread.OnThreadTerminate(Sender: TObject);
begin
  FMyId := (Sender as TMyThread).Id;
  FThreadDone := True;
end;

This should do the job.

EDIT: Corrected stupid corrections, tested, works.

alzaimar
  • 4,572
  • 1
  • 16
  • 30
  • The only place I am accessing the running thread is in the `OnTerminate` handler which is called *before* the thread is freed. Please note that i create the thread in a suspended state and manually resume it. – alzaimar Mar 20 '13 at 07:29
  • 2
    Resume is deprecated in newer Delphi versions. In those you should use Start after creating a thread in suspended state. – Marjan Venema Mar 20 '13 at 08:02
  • 1
    This code will **deadlock**. The `OnTerminate` event is called via `Synchronize`, which means the main thread needs to continue checking for new synchronized methods. The spin-wait loop calling `Sleep` doesn't do that, but `WaitFor` calls `CheckSynchronize` to avoid that problem. Spin-wait loops on Boolean values are precisely what events are for. – Rob Kennedy Mar 20 '13 at 13:41
  • That is interesting, thank you. I thought, 'OnTerminate' is called from the calling context. I will correct my attempt. – alzaimar Mar 20 '13 at 16:53
  • Your edit is no better. The thread can be gone before you call `WaitFor`. You need a real sync object. Either an event, or wait the thread handle as per my answer. – David Heffernan Mar 20 '13 at 19:23
  • You've lost sight of the original problem; the new code fixes nothing. In a free-on-terminate thread, you must not access the thread object after it has started running because it's possible the object has already freed itself. In particular, that means you cannot call `WaitFor` after calling `Start`. Also, `FThreadDone` is written twice, but never read, so why does it exist? – Rob Kennedy Mar 20 '13 at 19:24
  • Sorry for the confusion, folks. I tested it and now it works. It might still not be correct, but it works. – alzaimar Mar 20 '13 at 19:34
  • @RobKennedy. Do you think the answer I provided would also cause deadlock? – Rick Wheeler Mar 20 '13 at 22:48
2

Because you made the thread free itself upon termination then you have asked it to destroy all traces of itself as soon as it is done. Since you cannot exert influence on when it finishes, it is wrong to refer to anything inside the thread after you start it.

The solutions proposed by other, namely asking the thread to signal you when it terminates, are good. I personally would probably elect to do it that way. If you use an event as a signal then you can wait on that event.

However, there is another way to do it.

  1. Create the thread suspended.
  2. Duplicate the thread handle.
  3. Start the thread.
  4. Wait on the duplicated handle.

Because you own the duplicated handle, rather than the thread, you are safe to wait on it. It seems a little more complicated, but I suppose it avoids creating an extra synchronization object where one is not needed. Note that I'm not advocating this approach over the approach of using an event to signal completion.

Anyway, here's a simple demonstration of the idea.

{$APPTYPE CONSOLE}

uses
  SysUtils, Windows, Classes;

type
  TMyThread = class(TThread)
  protected
    procedure Execute; override;
  public
    destructor Destroy; override;
  end;

destructor TMyThread.Destroy;
begin
  Writeln('I''m dead!');
  inherited;
end;

procedure TMyThread.Execute;
begin
end;

var
  DuplicatedHandle: THandle;

begin
  with TMyThread.Create(True) do // must create suspended
  begin
    FreeOnTerminate := True;
    Win32Check(DuplicateHandle(
      GetCurrentProcess,
      Handle,
      GetCurrentProcess,
      @DuplicatedHandle,
      0,
      False,
      DUPLICATE_SAME_ACCESS
    ));
    Start;
  end;

  Sleep(500);
  Writeln('I''m waiting');
  if WaitForSingleObject(DuplicatedHandle, INFINITE)=WAIT_OBJECT_0 then
    Writeln('Wait succeeded');
  CloseHandle(DuplicatedHandle);
  Readln;
end.
David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • Thanks David, I haven't test your solution mainly due to lack of time at the moment. I've gone with Rob's 2nd answer for now, but when I upgrade from Delphi 2010 I'll also try the answer below using anonymous threads which looks really interesting. Thanks again. – Rick Wheeler Mar 26 '13 at 05:12
2

Here is an example using an anonymous thread.

  • An event (TSimpleEvent) is created
  • An anonymous thread executes the test thread and
  • Waits for the event, which signals in the OnTerminate handler of the test thread
  • The anonymous thread is on hold until executed with a WaitFor
  • The result was picked up by the OnTerminate handler

The important thing here is that the event is waited for in a thread. No dead-lock situation.


Uses
  SyncObjs;

type

  TMyThread = class(TThread)
  private
    FId : Integer;
  protected
    procedure Execute; override;
  public
    constructor Create( anInt : Integer);
    property Id : Integer read FId;
  end;

  TestTMyThread = class
  strict private
    FMyId: Integer;
    FMyEvent : TSimpleEvent;
    procedure OnThreadTerminate(Sender: TObject);
  protected
  public
    procedure TestMyThread;
  end;

{ TMyThread }

constructor TMyThread.Create(anInt : Integer);
begin
  inherited Create(True);
  FreeOnTerminate := True;
  FId := anInt;
end;

procedure TMyThread.Execute;
begin
  Inc(FId);
end;

procedure TestTMyThread.TestMyThread;
var
  AnonThread : TThread;
begin
  FMyEvent := TSimpleEvent.Create(nil,true,false,'');
  try
    AnonThread :=
      TThread.CreateAnonymousThread(
        procedure
        begin
          With TMyThread.Create(1) do
          begin
            OnTerminate := Self.OnThreadTerminate;
            Start;
          end;
          FMyEvent.WaitFor; // Wait until TMyThread is ready
        end
      );
    AnonThread.FreeOnTerminate := False;
    AnonThread.Start;

    AnonThread.WaitFor; // Wait here until test is ready
    AnonThread.Free;

    Assert(FMyId = 2); // Check result
  finally
    FMyEvent.Free;
  end;
end;

procedure TestTMyThread.OnThreadTerminate(Sender: TObject);
begin
  FMyId := (Sender as TMyThread).Id;
  FMyEvent.SetEvent; // Signal TMyThread ready
end;

Update, since Delphi-2010 does not have an anonymous thread class, here is an alternative which you can implement:

Type
  TMyAnonymousThread = class(TThread)
    private
      FProc : TProc;
    protected
      procedure Execute; override;
    public
      constructor Create(CreateSuspended,SelfFree: Boolean; const aProc: TProc);
  end;

constructor TMyAnonymousThread.Create(CreateSuspended,SelfFree: Boolean; 
  const aProc: TProc);
begin
  Inherited Create(CreateSuspended);
  FreeOnTerminate := SelfFree;
  FProc := aProc;
end;

procedure TMyAnonymousThread.Execute;
begin
  FProc();
end;
LU RD
  • 34,438
  • 5
  • 88
  • 296
  • Would love to try this, but it seems that anonymous threads came in with Delphi XE, but I'm still on Delphi 2010. Another reason to upgrade... Thanks for your answer, when I do upgrade I'll revisit and see if it works. – Rick Wheeler Mar 26 '13 at 05:11