views:

1011

answers:

2

TThread's resume method is deprecated in D2010. So, I thought it should now work like this:

TMyThread = class (TThread)
protected
  Execute; override;
public
  constructor Create;
end;
...

TMyThread.Create;
begin
  inherited Create (True);
  ...
  Start;
 end;

Unfortunately I get an exception "Cannot call start on a running or supsended thread"...which seems weird to me considering the fact that the documentation tells me that I should call Start on a thread created in suspended mode.

What am I missing here?

+7  A: 

Short answer: call inherited Create(false) and omitt the Start!

The actual Start of a non-create-suspended thread is done in AfterConstruction, which is called after all constructors have been called.

Uwe Raabe
Note that this is a relatively recent development. Older versions really would start running as soon as the inherited constructor finished running. There's a really easy workaround to that, though: Call the inherited constructor **last**. (There's little reason to call it first; a descendant rarely needs any of the property values that the base TThread constructor sets.)
Rob Kennedy
He mentioned D2010 explicitly...
Uwe Raabe
No, @Jan, the inherited constructor is not at liberty to set your fields to zero. It can't access your fields because it was written and compiled before those fields even existed, so it can't refer to them. (If you're thinking of the constructor using ZeroMemory to clear itself, that would be disastrous in *any* class, not just TThread. All the fields get zeroed before *any* constructor runs. See TObject.InitInstance.)
Rob Kennedy
+12  A: 

The reason is that a Thread is not supposed to start itself.

The thread never knows when initialization is complete. Construction is not the same as initialization (construction should always be short and exception free; further initialization is done after construction).

A similar situation is a TDataSet: no TDataSet constructor should ever call Open, or set Active := True.

See also this blog entry by Wings of Wind.

You should either:

  • Create the TMyThread suspended by calling Create(true) and perform the Start outside your TMyThread class
  • Create the TMyThread non-suspeneded, making sure the Create constructor does full initialization, and let TThread.AfterConstruction start the thread.

Explanation of TThread usage:

Basically, a thread should be just that: the encapsulation of the context on which code is executed.

The actual code (the business logic) that is executed should then be in other classes.

By decoupling those two, you gain a lot of flexibility, especially initiating your business logic from within multiple places (which is very convenient when writing unit tests!).

This is the kind of framework you could use for that:

unit DecoupledThreadUnit;

interface

uses
  Classes;

type
  TDecoupledThread = class(TThread)
  strict protected
    //1 called in the context of the thread
    procedure DoExecute; virtual;
    //1 Called in the context of the creating thread (before context of the new thread actualy lives)
    procedure DoSetUp; virtual;
    //1 called in the context of the thread right after OnTerminate, but before the thread actually dies
    procedure DoTearDown; virtual;
  protected
    procedure DoTerminate; override;
    procedure Execute; override;
  public
    constructor Create;
    procedure AfterConstruction; override;
  end;

implementation

constructor TDecoupledThread.Create;
begin
  // create suspended, so that AfterConstruction can call DoSetup();
  inherited Create(True);
end;

procedure TDecoupledThread.AfterConstruction;
begin
  // DoSetUp() needs to be called without the new thread in suspended state
  DoSetUp();
  // this will unsuspend the underlying thread
  inherited AfterConstruction;
end;

procedure TDecoupledThread.DoExecute;
begin
end;

procedure TDecoupledThread.DoSetUp;
begin
end;

procedure TDecoupledThread.DoTearDown;
begin
end;

procedure TDecoupledThread.DoTerminate;
begin
  inherited DoTerminate();
  // call DoTearDown on in the thread context right before it dies:
  DoTearDown();
end;

procedure TDecoupledThread.Execute;
begin
  // call DoExecute on in the thread context
  DoExecute();
end;

end.

You could even make it event based by something like this:

unit EventedThreadUnit;

interface

uses
  Classes,
  DecoupledThreadUnit;

type
  TCustomEventedThread = class(TDecoupledThread)
  private
    FOnExecute: TNotifyEvent;
    FOnSetUp: TNotifyEvent;
    FOnTearDown: TNotifyEvent;
  strict protected
    procedure DoExecute; override;
    procedure DoSetUp; override;
    procedure DoTearDown; override;
  public
    property OnExecute: TNotifyEvent read FOnExecute write FOnExecute;
    property OnSetUp: TNotifyEvent read FOnSetUp write FOnSetUp;
    property OnTearDown: TNotifyEvent read FOnTearDown write FOnTearDown;
  end;

  // in case you want to use RTTI
  TEventedThread = class(TCustomEventedThread)
  published
    property OnExecute;
    property OnSetUp;
    property OnTearDown;
  end;

implementation

{ TCustomEventedThread }

procedure TCustomEventedThread.DoExecute;
var
  TheOnExecute: TNotifyEvent;
begin
  inherited;
  TheOnExecute := OnExecute;
  if Assigned(TheOnExecute) then
    TheOnExecute(Self);
end;

procedure TCustomEventedThread.DoSetUp;
var
  TheOnSetUp: TNotifyEvent;
begin
  inherited;
  TheOnSetUp := OnSetUp;
  if Assigned(TheOnSetUp) then
    TheOnSetUp(Self);
end;

procedure TCustomEventedThread.DoTearDown;
var
  TheOnTearDown: TNotifyEvent;
begin
  inherited;
  TheOnTearDown := OnTearDown;
  if Assigned(TheOnTearDown) then
    TheOnTearDown(Self);
end;

end.

Or adapt it for DUnit TTestCase descendants like this:

unit TestCaseThreadUnit;

interface

uses
  DecoupledThreadUnit,
  TestFramework;

type
  TTestCaseRanEvent = procedure (Sender: TObject; const TestResult: TTestResult) of object;
  TTestCaseThread = class(TDecoupledThread)
  strict private
    FTestCase: TTestCase;
  strict protected
    procedure DoTestCaseRan(const TestResult: TTestResult); virtual;
    function GetTestCase: TTestCase; virtual;
    procedure SetTestCase(const Value: TTestCase); virtual;
  protected
    procedure DoExecute; override;
    procedure DoSetUp; override;
    procedure DoTearDown; override;
  public
    constructor Create(const TestCase: TTestCase);
    property TestCase: TTestCase read GetTestCase write SetTestCase;
  end;

implementation

constructor TTestCaseThread.Create(const TestCase: TTestCase);
begin
  inherited Create();
  Self.TestCase := TestCase;
end;

procedure TTestCaseThread.DoExecute;
var
  TestResult: TTestResult;
begin
  if Assigned(TestCase) then
  begin
    // this will call SetUp and TearDown on the TestCase
    TestResult := TestCase.Run();
    try
      DoTestCaseRan(TestResult);
    finally
      TestResult.Free;
    end;
  end
  else
    inherited DoExecute();
end;

procedure TTestCaseThread.DoTestCaseRan(const TestResult: TTestResult);
begin
end;

function TTestCaseThread.GetTestCase: TTestCase;
begin
  Result := FTestCase;
end;

procedure TTestCaseThread.SetTestCase(const Value: TTestCase);
begin
  FTestCase := Value;
end;

procedure TTestCaseThread.DoSetUp;
begin
  if not Assigned(TestCase) then
    inherited DoSetUp();
end;

procedure TTestCaseThread.DoTearDown;
begin
  if not Assigned(TestCase) then
    inherited DoTearDown();
end;

end.

--jeroen

Jeroen Pluimers
So the correct way would be `MyThread := TMyThread.Create` and then `MyThread.Start`, performing all lengthy initialization at the beginning of `Execute`? How are users of the class supposed to know that the thread needs to be started manually? (Documentation aside)
Smasher
Since users of the class is assumed to be fully versed in multithreaded programming, that should be a minor detail. Don't let a multithreaded class escape to people who're not ready to deal with all the nuances of multithreaded programming.
Lasse V. Karlsen
Actually, see my edit: there are two ways to do this. In general, a thread should not know about its initialization, as that is depending on factors outside the scope of that thread. I know that most thread classes are specific to a problem. But they should be split into two: the thread class itself that just executes 'code', and a business class that knows about what code should be executed and in what order (initialization, main block, finalization).
Jeroen Pluimers
There are 264 messages in that newsgroup thread. Could you please **summarize**? Of course a thread should know about its initialization, just as any other class should know about its own initialization. That's what constructors are for. At the time the constructor finishes running, the class should be fully ready to use. There's nothing wrong with a constructor raising an exception; the language is *designed* for that. In general, there's absolutely nothing wrong with a thread starting itself. The only danger is in early versions, and it's easy to work around. (Uwe's answer and my comment.)
Rob Kennedy
Why would you go through a local variable in `TCustomEventedThread.DoExecute` et al? This is a false sense of security, because the event could not only be reassigned, the object could be destroyed as well, invalidating the saved event handler. All in all I'd say that an interface based solution is the only way to safely separate thread class and business logic class, dues to lifetime management issues being taken care of. No need to reinvent that, OTL is already there, which goes a long way in abstracting these things.
mghie
@Ted: I wish that the Embarcadero forums server had easier ways of pointing to single messages. I'll try to summarize: The crux is that the thread cannot start until the AfterConstruction is being called. But even at that point, it does not know if initialization is complete: something else might want to set some properties. Hence it is best to decouple the thread from the business logic to be run. Then you can initialize that logic and pass the complete thing to your thread.
Jeroen Pluimers
@mghie: it is about locking. Either I should lock the execution complete DoExecute method, or save a local copy. Either has drawbacks (locking can cause deadlocks; keeping a local copy has a chance of the underlying object being freed). From experience: Even interface based solutions can have objects that can be freed/disposed to early, even in a managed world.
Jeroen Pluimers
+1 thanks for your detailed answer. Although I don't fully understand the reasons why it's so bad to integrate business logic into a thread.
Smasher
@Smasher: don't worry; you'll learn over time what the advantages of decoupling are.
Jeroen Pluimers