Watch, Follow, &
Connect with Us

For forums, blogs and more please visit our
Developer Tools Community.


Welcome, Guest
Guest Settings
Help

Thread: TThread creatiion question



Permlink Replies: 23 - Last Post: Feb 27, 2015 9:35 AM Last Post By: Markus Humm
Markus Humm

Posts: 5,113
Registered: 11/9/03
TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 17, 2015 11:15 AM
Hello,

below is a reduced version of a Unit I struggled a bit with this
morning. It contained a race condition and I seemed to have found a
solution to it, but as it's using the deprecated (as to the help)
functionality of TThread because the recommended one crashed I'm asking
myself if I really fixed the issue and why the recommended solution crashed.

Here's what originally happened and what I did:

1. the constructor had been called with false as parameter and before
the thread's execute had been run for the first time somebody had
called AddItem. That one posts a message to a WndProc the TThread
sets up. But if the execute has not yet run it's not yet set up so
the message never reaches the planned destination.

2. after finding that out I changed the constructor. I added that TEvent
and tried to wait for that getting signalled so that the constructor
call would block as long as the thread needed to get properly set up.
It only worked if I create the thread suspended and then start it
with resume.

Trysing to start it via Start (todays recommended mnethod as to
the help resulted in an exception that I can't start a thread this
way if it's not suspended (I already had passed true to the
inherited constructor!).

So why did the start result in an exception and why does it work now
only if the thread is being created suspended?

Another thing: TEvent's constructor can receive a security context (or
something like that). I'm currently passing nil. Does this do any harm?
I don't want to signal events to other users or other processes.

unit RaceCondition;
 
interface
 
uses
  System.Classes, System.SysUtils, System.Generics.Collections,
  Windows, Messages, System.SyncObjs, MMTimer_Stable;
 
const
  c_ItemAddTimeout    = 2000;
	
	wm_ThreadTerminate = wm_app+10;
	wm_ThreadAddItem   = wm_app+11;
 
type
  EItemAddTimeoutException = class(Exception);
	
	TMyItem = class(TObject)
	strict private
	  // something goes here
	public
	  // and here	
	end;
 
  TRaceConditionThread = class(TThread)
  strict private
    FState           : Boolean;
    FItemDict        : TDictionary<Byte, TMyItem>;
    FWindowHandle    : Cardinal;
    FCriticalSection : TCriticalSection;
    FEvent           : TEvent;
  strict protected
    procedure Execute; override;
    procedure WndProc(var msg: TMessage);
		procedure OnTimer(Sender: TObject; Time: Cardinal);
  public
    constructor Create(ACreateSuspended:Boolean);
	
    procedure Terminate;
    function AddItem(ItemKey : Byte) : TMyItem;
 
    property State : Boolean
      read   FState
      write  FState;
  end;
 
implementation
 
 
{ TMyThread }
 
constructor TFanModelThread.Create(ACreateSuspended: Boolean);
var
  StartTime : DWord;
  Res       : TWaitResult;
begin
  FWindowHandle   := 0;
  FItemDict       := TDictionary<Byte, TMyItem>.Create;
  FreeOnTerminate := true;
  FState          := false;
 
  FEvent          := TEvent.Create(nil, true, false,
                                   'RaceConditionThreadEvent', false);
 
  inherited Create(true);
 
  Resume;
 
  Res := FEvent.WaitFor(10000);
  if (Res <> wrSignaled) then
    ; // logging goes here
 
  FEvent.Free;
end;
 
procedure TFanModelThread.Execute;
var
  index     : Integer;
  Item      : TMyItem;
  Msg       : TMsg;
  CalcTimer : TMMTimerstable;
begin
  CalcTimer           := TMMTimerstable.Create(nil);
  CalcTimer.Interval  :=  c_TimerUpdateInterval;
  CalcTimer.Periodic  :=  true;
  CalcTimer.OnTimer   :=  OnTimer;
  CalcTimer.Enabled   :=  true;
 
  FCriticalSection    := TCriticalSection.create;
 
  FState              := true;
  FWindowHandle       := AllocateHwnd(WndProc);
 
  // Write to log via CodeSite
 
	FEvent.SetEvent;
 
  while GetMessage(Msg, FWindowHandle, 0, 0) do
  begin
    TranslateMessage(Msg);
    Dispatch(Msg);
 
    case msg.message of
       wm_ThreadTerminate :  begin
                               CalcTimer.OnTimer := nil;
                               CalcTimer.Enabled := false;
                               break;
                             end;
       wm_ThreadAddItem   :  begin
                               // Write value of FWindowHandle
                               // into the log via CodeSite
																		
                               if FItemDict.ContainsKey(msg.lParam) then
                               begin
                                 FItemDict.Items[msg.lParam].Free;
                                 FItemDict.Remove(msg.lParam);
                               end;
 
                               FItemDict.Add(msg.lParam,
                                             TMyItem.Create(
                                                c_TimerUpdateInterval,
                                                msg.lParam));
                             end;
    end;
  end;
 
  for index in FItemDict.Keys do
  begin
    Item := FItemDict.Items[index];
    if Assigned(Item) then
      Item.Free;
  end;
  FItemDict.Clear;
  FItemDict.Free;
  CalcTimer.Free;
  FCriticalSection.Free;
  DeallocateHWnd(FWindowHandle);
end;
 
 
procedure TFanModelThread.OnTimer(Sender: TObject; Time: Cardinal);
begin
  // snip contents
end;
 
procedure TFanModelThread.WndProc(var msg: TMessage);
begin
  if msg.msg = WM_SHOWWINDOW then
  {
   if the message id is WM_SHUTDOWN_THREADS
   do our own processing
  }
  //lShutdown := True
  else
  {
   for all other messages call
   the default window procedure
  }
  Msg.Result := DefWindowProc(FWindowHandle, Msg.Msg,
                              Msg.wParam, Msg.lParam);
end;
 
function TFanModelThread.AddItem(ItemKey : Byte) : TMyItem ;
var
  StartTime : DWord;
begin
  PostMessage(FWindowHandle, wm_ThreadAddItem, 0, ItemKey);
  // CodeSite logging of Window handle goes here
 
  StartTime := GetTickCount;
  while (FItemDict.ContainsKey(ItemKey) = false) and
        (abs(GetTickCount-StartTime) < c_ItemAddTimeout) do
  begin
    sleep(1);
  end;
 
  if (FItemDict.ContainsKey(ItemKey) = false) then
    raise EFanAddTimeoutException.Create('Timeout adding item '+
                                         'with key: '+ItemKey.ToString)
  else
    result := FItemDict.Items[ItemKey];
end;
 
procedure TFanModelThread.Terminate;
begin
  inherited;
  PostMessage(FWindowHandle, wm_ThreadTerminate, 0, 0);
end;
 
end.
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 17, 2015 11:59 AM   in response to: Markus Humm in response to: Markus Humm
From very quick look I see that you have public Terminate procedure
that hides TThread one. You should override virtual protected procedure
DoTerminate instead.

--
Dalija Prasnikar
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 17, 2015 2:22 PM   in response to: Dalija Prasnikar in response to: Dalija Prasnikar
Dalija wrote:

From very quick look I see that you have public Terminate procedure
that hides TThread one.

That is not uncommon. Even I do that sometimes.

You should override virtual protected procedure DoTerminate instead.

Overriding DoTerminate() is not the same as overriding/hiding Terminate().
Terminate() signals the thread to terminate itself. DoTerminate() is called
by TThread after Execute() exits. DoTerminate() is a good place to perform
thread-specific cleanup (like freeing the HWND that Execute() creates).

--
Remy Lebeau (TeamB)
Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 17, 2015 11:29 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Hello,

thanks for your replies, but those still leave my main question about the thread
creation / resume / start open.

Do you have any insight on that topic?

Greetings

Markus
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 1:06 AM   in response to: Markus Humm in response to: Markus Humm
Markus Humm wrote:
Hello,

thanks for your replies, but those still leave my main question about the thread
creation / resume / start open.

Do you have any insight on that topic?

There are some things that don't sound quite right in you code.
However, I am rather busy today and I am not sure if I can take
a deeper look.

--
Dalija Prasnikar
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 9:20 AM   in response to: Markus Humm in response to: Markus Humm
Markus Humm wrote:
Hello,

thanks for your replies, but those still leave my main question about the thread
creation / resume / start open.

Do you have any insight on that topic?

Ok, small update :)

Thread startup happens in AfterConstruction, after all constructors have
finished their work. So you don't have to create thread in suspended state to do
initialization in constructor. That is also probably causing some issues with your
call to Resume inside constructor.

Quick out of your trouble would be something like (removed all timeouts and
resume in constructor)


  TFanModelThread = class(TThread)
  protected
    procedure TerminatedSet; override; <- instead overriding Terminate
  public
    IsRunning: boolean; 
...
 
procedure TFanModelThread.Execute;
...
 
  IsRunning := true;
  while GetMessage(msg, FWindowHandle, 0, 0) do
...
 
  th := TFanModelThread.Create(false);
  while not th.IsRunning do
    Sleep(100);
  th.AddItem(0);
  th.AddItem(2);

I hope it will be descriptive enough.

--
Dalija Prasnikar

Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 9:51 AM   in response to: Dalija Prasnikar in response to: Dalija Prasnikar
Am 18.02.2015 um 18:20 schrieb Dalija Prasnikar:
Markus Humm wrote:

 
  TFanModelThread = class(TThread)
  protected
    procedure TerminatedSet; override; <- instead overriding Terminate
  public
    IsRunning: boolean; 
...
 
procedure TFanModelThread.Execute;
...
 
  IsRunning := true;
  while GetMessage(msg, FWindowHandle, 0, 0) do
...
 
  th := TFanModelThread.Create(false);
  while not th.IsRunning do
    Sleep(100);
  th.AddItem(0);
  th.AddItem(2);



Hello,

I had something similiar to this already tested, but I had it placed
inside of the TFanModelThread.Create and there it deadlocked.

Your idea is now to wait at the place the thread object reference is
being created.

Greetings

Markus
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 12:59 PM   in response to: Markus Humm in response to: Markus Humm
Markus Humm wrote:
Am 18.02.2015 um 18:20 schrieb Dalija Prasnikar:
Markus Humm wrote:

 
  TFanModelThread = class(TThread)
  protected
    procedure TerminatedSet; override; <- instead overriding Terminate
  public
    IsRunning: boolean; 
...
 
procedure TFanModelThread.Execute;
...
 
  IsRunning := true;
  while GetMessage(msg, FWindowHandle, 0, 0) do
...
 
  th := TFanModelThread.Create(false);
  while not th.IsRunning do
    Sleep(100);
  th.AddItem(0);
  th.AddItem(2);



Hello,

I had something similiar to this already tested, but I had it placed
inside of the TFanModelThread.Create and there it deadlocked.

Your idea is now to wait at the place the thread object reference is
being created.

You can also put it in AfterConstruction after call to inherited that will
actually start the thread. It should work there, too.

--
Dalija Prasnikar
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 19, 2015 7:48 AM   in response to: Markus Humm in response to: Markus Humm
Markus Humm wrote:
Am 18.02.2015 um 18:20 schrieb Dalija Prasnikar:
Markus Humm wrote:

 
  TFanModelThread = class(TThread)
  protected
    procedure TerminatedSet; override; <- instead overriding Terminate
  public
    IsRunning: boolean; 
...
 
procedure TFanModelThread.Execute;
...
 
  IsRunning := true;
  while GetMessage(msg, FWindowHandle, 0, 0) do
...
 
  th := TFanModelThread.Create(false);
  while not th.IsRunning do
    Sleep(100);
  th.AddItem(0);
  th.AddItem(2);



Hello,

I had something similiar to this already tested, but I had it placed
inside of the TFanModelThread.Create and there it deadlocked.

Your idea is now to wait at the place the thread object reference is
being created.

I posted some code with question on SO, you can get some ideas from
there too. http://stackoverflow.com/a/28608607/4267244

I am currently rewriting some old code that could use something like that
so your post here came in perfect time for me too :)

--
Dalija Prasnikar
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 12:55 AM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:
Dalija wrote:

From very quick look I see that you have public Terminate procedure
that hides TThread one.

That is not uncommon. Even I do that sometimes.

You should override virtual protected procedure DoTerminate instead.

Overriding DoTerminate() is not the same as overriding/hiding Terminate().
Terminate() signals the thread to terminate itself. DoTerminate() is called
by TThread after Execute() exits. DoTerminate() is a good place to perform
thread-specific cleanup (like freeing the HWND that Execute() creates).

You are right about DoTerminate, I wanted to say he should override TerminatedSet.

But, hiding Terminate is not very good idea because it is not virtual and you can
end up with your method not being called at all.

  TMyThread = class(TThread)
  public
    procedure Terminate;
  end;
 
var
  t: TThread;
 
  t := TMyThread.Create;
...
  t.Terminate; // will only execute TThread.Terminate and not your method


--
Dalija Prasnikar
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 11:42 AM   in response to: Dalija Prasnikar in response to: Dalija Prasnikar
Dalija wrote:

You are right about DoTerminate, I wanted to say he should override
TerminatedSet.

That makes more sense. Just note that TerminatedSet() was introduced in XE2.

But, hiding Terminate is not very good idea because it is not virtual
and you can end up with your method not being called at all.

Yes, but only if you are dealing with general TThread pointers instead of
specific TMyThread pointers.

--
Remy Lebeau (TeamB)
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 1:02 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:
Dalija wrote:

You are right about DoTerminate, I wanted to say he should override
TerminatedSet.

That makes more sense. Just note that TerminatedSet() was introduced in XE2.

Noted. But, I think Markus is using later versions.

But, hiding Terminate is not very good idea because it is not virtual
and you can end up with your method not being called at all.

Yes, but only if you are dealing with general TThread pointers instead of
specific TMyThread pointers.

True.

--
Dalija Prasnikar
Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 1:23 PM   in response to: Dalija Prasnikar in response to: Dalija Prasnikar
Am 18.02.2015 um 22:02 schrieb Dalija Prasnikar:
Remy Lebeau (TeamB) wrote:
Dalija wrote:

You are right about DoTerminate, I wanted to say he should override
TerminatedSet.

That makes more sense. Just note that TerminatedSet() was introduced in XE2.

Noted. But, I think Markus is using later versions.

Yes and no ;-)
But yes: this project is fortunately already on XE6.
My big main application is still on D2007 :-(
(and the conversion to XE7 just got postphones a little bit further
[hopefully only weeks] by current events :-( )

I didn't know yet about the TerminatedSet.
Maybe I get a little time for all of this tomorrow, as I'm still working
on that project (the current work I'm doing on it is a bit of a gap
filler, as I'm waiting on something from a colleague which will have
priority then, but I can fill the gap with TThread things now ;-) )

Greetings

Markus
Dalija Prasnikar

Posts: 2,325
Registered: 11/9/99
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 19, 2015 5:16 AM   in response to: Markus Humm in response to: Markus Humm
Markus Humm wrote:
Am 18.02.2015 um 22:02 schrieb Dalija Prasnikar:
Remy Lebeau (TeamB) wrote:
Dalija wrote:

You are right about DoTerminate, I wanted to say he should override
TerminatedSet.

That makes more sense. Just note that TerminatedSet() was introduced in XE2.

Noted. But, I think Markus is using later versions.

Yes and no ;-)
But yes: this project is fortunately already on XE6.
My big main application is still on D2007 :-(
(and the conversion to XE7 just got postphones a little bit further
[hopefully only weeks] by current events :-( )

I didn't know yet about the TerminatedSet.

Not having TerminatedSet should not be an issue as long as you don't store
thread variable in generic TThread reference, just like Remy said.

BTW, I forgot to mention that AllocateHWnd and DeallocateHWnd are not thread
safe. Depending on the rest of your code you can run into some hard to track issues
there.

AllocateHwnd is not thread-safe
http://qc.embarcadero.com/wc/qcmain.aspx?d=47559

--
Dalija Prasnikar
Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 19, 2015 9:21 AM   in response to: Dalija Prasnikar in response to: Dalija Prasnikar
Am 19.02.2015 um 14:16 schrieb Dalija Prasnikar:
Markus Humm wrote:
Am 18.02.2015 um 22:02 schrieb Dalija Prasnikar:
Remy Lebeau (TeamB) wrote:
Dalija wrote:

You are right about DoTerminate, I wanted to say he should override
TerminatedSet.

That makes more sense. Just note that TerminatedSet() was introduced in XE2.

Noted. But, I think Markus is using later versions.

Yes and no ;-)
But yes: this project is fortunately already on XE6.
My big main application is still on D2007 :-(
(and the conversion to XE7 just got postphones a little bit further
[hopefully only weeks] by current events :-( )

I didn't know yet about the TerminatedSet.

Not having TerminatedSet should not be an issue as long as you don't store
thread variable in generic TThread reference, just like Remy said.

BTW, I forgot to mention that AllocateHWnd and DeallocateHWnd are not thread
safe. Depending on the rest of your code you can run into some hard to track issues
there.

AllocateHwnd is not thread-safe
http://qc.embarcadero.com/wc/qcmain.aspx?d=47559

Hello,

what does this mean to me? I need to protect it with a TCriticalSection
or wasn't there some modern equivalent/replacement from System.pas?
(I forgot it's name)

Greetings

Markus
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 19, 2015 10:37 AM   in response to: Markus Humm in response to: Markus Humm
Markus wrote:

what does this mean to me?

It means you cannot use AllocateHWnd()/DeallocateHwnd() in your thread at
all, or else you risk corrupting memory.

I need to protect it with a TCriticalSection

That is not adequate enough, because the main thread will not use your TCriticalSection
when it uses AllocateHWnd()/DeallocateHWnd() for its own purposes.

--
Remy Lebeau (TeamB)
Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 19, 2015 11:28 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:
Markus wrote:

what does this mean to me?

It means you cannot use AllocateHWnd()/DeallocateHwnd() in your thread at
all, or else you risk corrupting memory.

I need to protect it with a TCriticalSection

That is not adequate enough, because the main thread will not use your TCriticalSection
when it uses AllocateHWnd()/DeallocateHWnd() for its own purposes.

Hello,

that's some important information. I guess in my case the creation of a window for the thread is
superflous if I use PostThreadMessage instead of PostMessage for my decoupling. Correct?

Greetings

Markus
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 20, 2015 12:20 AM   in response to: Markus Humm in response to: Markus Humm
Markus wrote:

that's some important information. I guess in my case the creation of
a window for the thread is superflous if I use PostThreadMessage
instead of PostMessage for my decoupling. Correct?

Correct. You do not need the HWND or the WndProc() at all.

Try something more like this:

unit RaceCondition;
 
interface
 
uses
  System.Classes, System.SysUtils, System.Generics.Collections,
  Windows, Messages, System.SyncObjs, MMTimer_Stable;
 
type
  EItemException = class(Exception);
  EItemAddException = class(EItemException);
  EItemAddTimeoutException = class(EItemException);
 
  TMyItem = class(TObject)
  strict private
    // something goes here
  public
    // and here    
  end;
 
  TRaceConditionThread = class(TThread)
  strict private
    FState : Boolean;
    FItemDict : TObjectDictionary<Byte, TMyItem>;
    FCriticalSection : TCriticalSection;
    FEvent : TEvent;
  strict protected
    procedure AfterConstruction; override;
    procedure Execute; override;
    procedure DoTerminate; override;
    procedure TerminatedSet; override;
    procedure OnTimer(Sender: TObject; Time: Cardinal);
  public
    constructor Create; reintroduce;
    destructor Destroy; override;
 
    function AddItem(ItemKey : Byte) : TMyItem;
 
    property State : Boolean read FState write FState;
  end;
 
implementation
 
const
  c_ItemAddTimeout = 2000;
 
  WM_ThreadAddItem = WM_APP+10;
 
{ TMyThread }
 
constructor TFanModelThread.Create;
begin
  inherited Create(False);
  FreeOnTerminate := true;
  FState := false;
  FItemDict := TObjectDictionary<Byte, TMyItem>.Create([doOwnsValues]);
  FCriticalSection := TCriticalSection.Create;
  FEvent := TEvent.Create(nil, true, false, '', false);
end;
 
destructor TFanModelThread.Destroy;
begin
  FItemDict.Free;
  FEvent.Free;
  FCriticalSection.Free;
  inherited;
end;
 
procedure TFanModelThread.AfterConstruction;
var
  Res : TWaitResult;
begin
  inherited;
  Res := FEvent.WaitFor(10000);
  if (Res <> wrSignaled) or (not FState) then
  begin
    // logging goes here
  end;
end;
 
procedure TFanModelThread.Execute;
var
  Item : TMyItem;
  Msg : TMsg;
  CalcTimer : TMMTimerstable;
begin
  CalcTimer := TMMTimerstable.Create(nil);
  try
    CalcTimer.Interval := c_TimerUpdateInterval;
    CalcTimer.Periodic := true;
    CalcTimer.OnTimer := OnTimer;
    CalcTimer.Enabled := true;
    try
      FState := True;
      FEvent.SetEvent;
 
      while GetMessage(Msg, 0, 0, 0) and (not Terminated) do
      begin
        if Msg.message = WM_ThreadAddItem then
        begin
          FCriticalSection.Enter;
          try
            Item := TMyItem.Create(c_TimerUpdateInterval, msg.lParam);
            try
              FItemDict.AddOrSetValue(msg.lParam, Item);
            except
              Item.Free;
              raise;
            end;
          finally
            FCriticalSection.Leave;
          end;
        end;
      end;
    finally
      CalcTimer.Enabled := false;
      CalcTimer.OnTimer := nil;
    end;
  finally
    CalcTimer.Free;
  end;
end;
 
procedure TFanModelThread.DoTerminate;
begin
  if (FatalException <> nil) then
  begin
    // logging goes here
  end;
  FEvent.SetEvent; // in case AfterConstruction() is still waiting...
  FCriticalSection.Enter;
  try
    FItemDict.Clear;
  finally
    FCriticalSection.Leave;
  end;
  inherited;
end;
 
procedure TFanModelThread.OnTimer(Sender: TObject; Time: Cardinal);
begin
  // snip contents
end;
 
function TFanModelThread.AddItem(ItemKey : Byte) : TMyItem ;
var
  StartTime : DWord;
begin
  Result := nil;
 
  if not PostThreadMessage(ThreadID, WM_ThreadAddItem, 0, ItemKey) then
  begin
    try
      RaiseLastOSError;
    except
      Exception.RaiseOuterException(EItemAddException.Create('Error adding 
item with key: '+ItemKey.ToString));
    end;
  end;
 
  StartTime := GetTickCount;
  repeat
    FCriticalSection.Enter;
    try
      if FItemDict.TryGetValue(ItemKey, Result) then
        Exit;
    finally
      FCriticalSection.Leave;
    end;
    Sleep(1);
  until (abs(GetTickCount-StartTime) >= c_ItemAddTimeout);
 
  raise EFanAddTimeoutException.Create('Timeout adding item with key: '+ItemKey.ToString);
end;
 
procedure TFanModelThread.TerminatedSet;
begin
  PostThreadMessage(ThreadID, WM_QUIT, 0, 0);
end;
 
end.


--
Remy Lebeau (TeamB)
Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 26, 2015 1:16 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Hello,

I tried your solution in an adapted version and first tests indicate
that it works as it should.

One thing in your code is wrong though: TDictionary<T> has no concept of
object ownership. The constructor you're trying to use doesn't exist in XE6.

While I'm tempted to request enhancement of that class with object
ownership concept I doubt they'll agree to do it, as afaik the key could
be an object as well and that creates quite a few combinations of what
to own and not and that class already has 4 or more constructors.
I don't think they'd like to double or so that number ;-)

Greetings

Markus
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 26, 2015 2:08 PM   in response to: Markus Humm in response to: Markus Humm
Markus wrote:

One thing in your code is wrong though: TDictionary<T> has no concept
of object ownership. The constructor you're trying to use doesn't
exist in XE6.

Look at my example again more carefully. It uses TObjectDictionary, not
TDictionary:

FItemDict : TObjectDictionary<Byte, TMyItem>;
...
FItemDict := TObjectDictionary<Byte, TMyItem>.Create([doOwnsValues]);


TObjectDictionary supports object ownership.

While I'm tempted to request enhancement of that class with object
ownership concept

It already exists, via TObjectDictionary.

I doubt they'll agree to do it, as afaik the key could be an object as well
and that creates quite a few combinations of what to own and not

TObjectDictionary supports object ownership on both keys and values, and
there are separate flags so you can specify what is owned and what is not
owned.

and that class already has 4 or more constructors.

So?

I don't think they'd like to double or so that number ;-)

They don't need to. Multiple flags can be specified in a single parameter,
and the 3 new constructors introduced by TObjectDictionary have that parameter.

--
Remy Lebeau (TeamB)
Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 27, 2015 9:33 AM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Am 26.02.2015 um 23:08 schrieb Remy Lebeau (TeamB):
Markus wrote:

One thing in your code is wrong though: TDictionary<T> has no concept
of object ownership. The constructor you're trying to use doesn't
exist in XE6.

Look at my example again more carefully. It uses TObjectDictionary, not
TDictionary:

Hello,

ok, you're right of course. I had missed that little item.

Greetings

Markus
Markus Humm

Posts: 5,113
Registered: 11/9/03
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 27, 2015 9:35 AM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Am 26.02.2015 um 23:08 schrieb Remy Lebeau (TeamB):
Markus wrote:

One thing in your code is wrong though: TDictionary<T> has no concept
of object ownership. The constructor you're trying to use doesn't
exist in XE6.

Look at my example again more carefully. It uses TObjectDictionary, not
TDictionary:

Hello,

ok, you're right of course. I had missed that little item.

Greetings

Markus
Olivier Sannier

Posts: 424
Registered: 8/26/01
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 18, 2015 11:40 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:

But, hiding Terminate is not very good idea because it is not virtual
and you can end up with your method not being called at all.

Yes, but only if you are dealing with general TThread pointers instead of
specific TMyThread pointers.

Which is exactly what TThread.Destroy is doing...
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: TThread creatiion question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Feb 19, 2015 10:35 AM   in response to: Olivier Sannier in response to: Olivier Sannier
Olivier wrote:

Which is exactly what TThread.Destroy is doing...

Only if the thread is still running when you free the TThread. You should
never free a running TThread, always terminate it explicitly first.

--
Remy Lebeau (TeamB)
Legend
Helpful Answer (5 pts)
Correct Answer (10 pts)

Server Response from: ETNAJIVE02