Hi All,

Well, caffeine was what I needed. I've managed to get a working
prototype in action - and that before lunch!!!

I've placed the prototype in the binaries newsgroup.

I ended up having 3 units in total:

1) MainUnit (which would be the form that would call the thread)

2) MyThreadUnit - which is the unit that contains the entire thread
stuff, and

3) ThreadCheckDM - this is an independent Datamodule that contains the
procedure required to do the validation check.


I broke it up like this because I figure that I could use the
ThreadCheckDM outside of a thread if I want independently, and then just
throw in the threaded stuff later on. (This way I could turn threading
on or off within my application if I needed to until I'm satisfied with
the results). It also made it easier for me to see it all 'split up'
than in the one unit.

Where I'm not sure is:

1) DataReady Boolean property. I'm not sure how / why this is required
now that I've written the thread, so I'm concerned that I've done
something very wrong.

2) I've ended up using Raul's approach to use the OnTerminate event to
handle when dialogs show. However, I'm not sending any messages back to
the main form through a handle. Is this OK? I figured that since the
OnThreadFinish won't execute until the ReturnMessage string is populated
I should be safe to view it (as it's a unidirectional message)?

3) I haven't put anything in the main unit yet that will terminate the
thread if the main form terminates. I wasn't sure exactly how to go
about doing that. Should the main form 'wait' until the thread is
terminated before closing itself?

Thanks again for all the help!

Adam.

Adam


Its not downloaded on the binaries here but it is on the web version - strange

Roy Lambert

> 1) DataReady Boolean property. I'm not sure how / why this is
> required now that I've written the thread, so I'm concerned that
> I've done something very wrong.
>
> 2) I've ended up using Raul's approach to use the OnTerminate event
> to handle when dialogs show.

I've not looked at your code yet, but the latter is why the former doesn't get used.
Plus termination is affected. What it comes down to is the lifetime of the form
that is asking for the work to be done.

If you have the thread call the form using OnTerminate or other such, then you have
to ensure that the form waits for the thread to terminate before you can close the
form. This isn't hard - the OnClose just has to call Terminate and then wait for it
to finish. The thread has to be coded to check if it is Terminated, and if so, get
out quickly.

if assigned(m_xMaintenanceThread) then
begin
 m_xMaintenanceThread.Terminate;
 m_xMaintenanceThread.WaitFor;
 FreeAndNil(m_xMaintenanceThread);
end;


If you have the thread go away and do the work, but the form uses a timer to check
if the answer is ready, then the form can just close any time, and the thread will
finish and die independently, unaware that the results were never used. The global
variable with the data is still sitting there happily for both.

The idea of the data module is good, and does indeed support the mode you suggest.
I usually start my background thread code in a normal application, then move the
code to a separate unit with a new class to manage it. Then I just create a thread
that runs the class code. The data module allows you to use the components for
databases more easily.

/Matthew Jones/

Adam


Its arrived. There is much about it that I would do differently.

Minor point

MF.FreeOnTerminate := true; isn't needed since you also set it in the thread's creator.

You're passing actual sessions & databases around and this may not be a good thing. Remember the rule is total isolation and this breaks the rule. Just pass the needed information to create you own session and database where needed.

Maybe I'm showing my ignorance with this one

procedure TMyThread.DoHandleException;
begin
 // Cancel the mouse capture
 if GetCapture <> 0 then
   SendMessage(GetCapture, WM_CANCELMODE, 0, 0);
 // Now actually show the exception
 if FException is Exception then
   Application.ShowException(FException)
 else
   SysUtils.ShowException(FException, nil);

end;

The thread runs in the background it should have no interaction with mice. Also I, personally, am very unhappy with anything that interacts with the screen in a thread. I know Raul says MessageBox is safe but I'd still avoid it. Also you're terminating on the exception so just PostMessage the error number back to the main thread and let it notify the user.

Another point is that your Execute code is a single pass so once an exception hits it unless you process that exception and return to some point in the code you're out of it. Terminate - who cares - its dead anyway.

OnTerminate is strange. Firstly I wouldn't use it, secondly its already a part of the TThread class and by redeclaring it in your definition you've removed that link. Get rid of its declartion in your thread and calling it in Execute and let it do its own thing. Personally I'd use PostMessage to send the info back to the main thread.


Why not use the constructor and destructor to create / free the datamodule?

Bit rambling but that's what a quick run through gave me.

Roy Lambert

Hi Matthew,

Thanks for your help...

> If you have the thread call the form using OnTerminate or other such, then you have
> to ensure that the form waits for the thread to terminate before you can close the
> form. This isn't hard - the OnClose just has to call Terminate and then wait for it
> to finish. The thread has to be coded to check if it is Terminated, and if so, get
> out quickly.
>
>   if assigned(m_xMaintenanceThread) then
>   begin
>    m_xMaintenanceThread.Terminate;
>    m_xMaintenanceThread.WaitFor;
>    FreeAndNil(m_xMaintenanceThread);
>   end;
>
> If you have the thread go away and do the work, but the form uses a timer to check
> if the answer is ready, then the form can just close any time, and the thread will
> finish and die independently, unaware that the results were never used. The global
> variable with the data is still sitting there happily for both.

So either way, I should be declaring TMyThread as part of the Form's
Variables, not as part of the Button1Click's variable?

Thanks & Regards

Adam.

Hi Roy,

Thanks again for your help...

> You're passing actual sessions & databases around and this may not be a good thing. Remember the rule is total isolation and this breaks the rule. Just pass the needed information to create you own session and database where needed.

My thought was that it was easier / simpler to pass the database and
sessions so I could grab all the properties I needed later, than to have
a seperate variable to pass for each one. I suppose I could create a new
packed record to contain the information.

While it's not a good thing - is it safe to do provided it's one way /
uni directional (like the other variables)?

> Maybe I'm showing my ignorance with this one
>
> procedure TMyThread.DoHandleException;
> begin
>    // Cancel the mouse capture
>    if GetCapture <> 0 then
>      SendMessage(GetCapture, WM_CANCELMODE, 0, 0);
>    // Now actually show the exception
>    if FException is Exception then
>      Application.ShowException(FException)
>    else
>      SysUtils.ShowException(FException, nil);
>
> end;

I found this code somewhere on the net when I was trying to deal with
exceptions within a thread. I don't fully understand it (the same way I
don't fully understand threads , so I'm not sure what the mouse
capture relates to.


> I know Raul says MessageBox is safe but I'd still avoid it. Also you're terminating on the exception so just PostMessage the error number back to the main thread and let it notify the user.

Would that be as simple as:

procedure TMyThread.DoHandleException;
begin
PostMessage(HWND, FException, 0, 0);
terminate;
end;

....or would I need to write additional code for the message handler?
(I'm really doing guesswork with message handling. I've tried to learn
it on numerous occasions but it's something that seems to evade my
understanding).


> Another point is that your Execute code is a single pass so once an exception hits it unless you process that exception and return to some point in the code you're out of it. Terminate - who cares - its dead anyway.

So - adding the terminate in the DoHandleException routine is the way to go?

> OnTerminate is strange. Firstly I wouldn't use it, secondly its already a part of the TThread class and by redeclaring it in your definition you've removed that link. Get rid of its declartion in your thread and calling it in Execute and let it do its own thing. Personally I'd use PostMessage to send the info back to the main thread.

Thanks for picking up that the additional declaration isn't needed. As
for why I chose to use it, I guess I used it as I understood it, as
opposed to PostMessage where I feel like I'm groping in the dark for
something to hand onto.

> Why not use the constructor and destructor to create / free the datamodule?

Which constructor is that? The threads constructor? Probably once again
something that I haven't been familiar with in the past. Is there any
benefits doing it that way?

Thanks & Regards

Adam.

> So either way, I should be declaring TMyThread as part of the
> Form's Variables, not as part of the Button1Click's variable?

Hmm, that's a deep question with lots of danger! The variable should be around as
long as you need it. If the thread is self-terminating, and it isn't going to do
anything external, then it could be anywhere. The OnButtonClick might be fine for a
set and forget click, but if the thread is going to "talk to" the form in any way,
then how would you know if it was finished? This is why I use a task controller
object that is "global". The button fills in a task object, gives it to the task
controller, and that gets a thread working on the task. It depends on how you go
about things.

/Matthew Jones/

> My thought was that it was easier / simpler to pass the database
> and sessions so I could grab all the properties I needed later,
> than to have a seperate variable to pass for each one. I suppose I
> could create a new packed record to contain the information.
>
> While it's not a good thing - is it safe to do provided it's one
> way / uni directional (like the other variables)?

While it may be safe, it opens up the possibility of accidentally using the main
thread components directly. You should work hard to keep a "firewall" between your
threads and anything else, particularly the main thread and VCL.

> o I'm not sure what the mouse
> capture relates to.

Try dragging a file in Windows Explorer, and then press Esc. The drag is cancelled.
Basically, the mouse gets "captured" by one application so that it can be in
control when you are dragging over other applications (otherwise it doesn't see
those messages outside its windows). Normally the mouse capture is ended when you
release the mouse button, probably dropping the item onto something. Cancelling the
capture is something you do when an action occurs that really kills the capture -
like the disk has an error, or some other activity occurs that should stop the
drag.

/Matthew Jones/

Adam

Roy Lambert

"Adam H." <ahairsub5@removeme.jvxp.com> wrote on Mon, 19 May 2014 10:50:16 +1000

>Hi Roy,
>
>Thanks again for your help...
>
>> You're passing actual sessions & databases around and this may not be a good thing. Remember the rule is total isolation and this breaks the rule. Just pass the needed information to create you own session and database where needed.
>
>My thought was that it was easier / simpler to pass the database and
>sessions so I could grab all the properties I needed later, than to have
>a seperate variable to pass for each one. I suppose I could create a new
>packed record to contain the information.

>While it's not a good thing - is it safe to do provided it's one way /
>uni directional (like the other variables)?

First - what Matthew says Second I try to follow threads on threads on the emb ngs and in one of them it talked about dangers of doing this. It seems as though you're safe but what happens if the main thread changes a variable just as you're reading it without benefit of a critical section or some other serialisation method. The answer is "most of the time fine - sometimes WOOPS"

Try and isolate everything to do with the thread, get into the habit and its a lot safer.


>> Maybe I'm showing my ignorance with this one
>>
>> procedure TMyThread.DoHandleException;
>> begin
>> // Cancel the mouse capture
>> if GetCapture <> 0 then
>> SendMessage(GetCapture, WM_CANCELMODE, 0, 0);
>> // Now actually show the exception
>> if FException is Exception then
>> Application.ShowException(FException)
>> else
>> SysUtils.ShowException(FException, nil);
>>
>> end;
>
>I found this code somewhere on the net when I was trying to deal with
>exceptions within a thread. I don't fully understand it (the same way I
>don't fully understand threads , so I'm not sure what the mouse
>capture relates to.

I'd dump it

>> I know Raul says MessageBox is safe but I'd still avoid it. Also you're terminating on the exception so just PostMessage the error number back to the main thread and let it notify the user.
>
>Would that be as simple as:
>
>procedure TMyThread.DoHandleException;
>begin
>PostMessage(HWND, FException, 0, 0);
>terminate;
>end;
>
>...or would I need to write additional code for the message handler?
>(I'm really doing guesswork with message handling. I've tried to learn
>it on numerous occasions but it's something that seems to evade my
>understanding).

Well you need to write a bit of code in the main thread to handle the message when it arrives but that's all. These days because (posters I respect on the emb ngs tell me) its possible for a forms handle to be changed I have a special handler in the main thread (or whereever I want the response from) to code with messages. I'll see if I can alter your example tonight and post to the binaries.

>
>> Another point is that your Execute code is a single pass so once an exception hits it unless you process that exception and return to some point in the code you're out of it. Terminate - who cares - its dead anyway.
>
>So - adding the terminate in the DoHandleException routine is the way to go?

Not needed as you have it structured. You have a single pass. When an exception is hit it will go to the exception handler and from there pass out of the Execute function and terminate itself.
>
>> OnTerminate is strange. Firstly I wouldn't use it, secondly its already a part of the TThread class and by redeclaring it in your definition you've removed that link. Get rid of its declartion in your thread and calling it in Execute and let it do its own thing. Personally I'd use PostMessage to send the info back to the main thread.
>
>Thanks for picking up that the additional declaration isn't needed. As
>for why I chose to use it, I guess I used it as I understood it, as
>opposed to PostMessage where I feel like I'm groping in the dark for
>something to hand onto.
>
>> Why not use the constructor and destructor to create / free the datamodule?
>
>Which constructor is that? The threads constructor? Probably once again
>something that I haven't been familiar with in the past. Is there any
>benefits doing it that way?

Yup - threads have constructors and destructors like every other class.

Roy Lambert

On 5/15/2014 9:35 PM, Adam H. wrote:
> 1) DataReady Boolean property. I'm not sure how / why this is required
> now that I've written the thread, so I'm concerned that I've done
> something very wrong.
>

It's not at all.

> 3) I haven't put anything in the main unit yet that will terminate the
> thread if the main form terminates. I wasn't sure exactly how to go
> about doing that. Should the main form 'wait' until the thread is
> terminated before closing itself?
You have 2 options :
- either wait til thread exits
- keep the thread reference around in the main thread and then you can
call terminate on it and waitfor (but you also need to check hew
terminate flag in the actual thread and DM and exit once its set.


Anyways i think your code had some issues so i've taken your code and
re-posted 2 samples back:
- 1a uses the onterminate
- 1b uses messages (which is generally better)

Both are light on error handling.

Couple of other notes :
- passing session and DB objects around the way you too is legal but
IMHO a bad design. You're basically creating bunch of potential bugs here :
- what if either of the objects gets freed before execute runs?
- since you're using components (instead of dynamically creating them) 1
year from now you're going to change something and go and and modify the
components on datamodule and then wonder why nothing changes
- there might be others

- You're calling resume both in thread constructor and from main thread.
In general you should not do it in the thread at all since
CreateSuspended is exactly for that - meaning if it's TRUE you must
assume calling function knows what it's doing and will call resume
itself (otherwise if it's false the TThread base class will resume it
anwyays)

- DoHandleException thing is not needed as it is - that is for form
window. In general execute function is where the exception handling is
needed

- OnTerminate - don't do this. it's already handled for you by tthread
- also don't call OnTerminate in execute - again done for you by base class

Raul