Does this thread code look OK

J

Just Me

Trying threads for the first time.
This seems to hang and I wonder if it looks OK to you.
Right now the thread t is simply a Form with a Cancel Button.
If clicked it should raise an event that sets mCancelled to true in the main
thread.

---The main thread contains
Private Sub UserCancelled(ByVal sender As Object, ByVal e As EventArgs)
mCancelled = True
End Sub

Private Function SetupAndPreview() As Boolean
SNIP
Dim mCancelled = False
Dim ch As New FormCheckForCancel
ch.Show()
Dim t As New Thread(AddressOf ch.AskIfCancel)
t.Start()
AddHandler ch.Cancelled, AddressOf UserCancelled
SNIP

--The t thread code is a Form
Public Class FormCheckForCancel
Inherits System.Windows.Forms.Form
SNIP

Public Event Cancelled(ByVal sender As Object, ByVal e As EventArgs)

Public Sub AskIfCancel()
Do
Application.DoEvents()
System.Threading.Thread.Sleep(500)
Loop

End Sub

Private Sub ButtonCancel_Click(ByVal sender As Object, ByVal e As
System.EventArgs) Handles ButtonCancel.Click
RaiseEvent Cancelled(Me, Nothing)
Me.Close()
End Sub
End Class
 
O

OpticTygre

The first thing that jumps out at me is the variable m_Cancelled. You Dim
it in one function, but try to change the value of it in another sub. You
may want to declare it outside your subroutines and functions so it can be
accessible to everything.
 
J

Just Me

I'm sorry, I tried to make the code short and introduced a bug. I do have:
Friend mCancelled As Boolean

Is it true that using a Form for the thread is OK?

Thanks
 
J

Just Me

Just Me said:
I'm sorry, I tried to make the code short and introduced a bug. I do have:
Friend mCancelled As Boolean
and no Dim on mCancelled = False
I removed it below
 
O

OpticTygre

That, I'm not sure of. I've never tried it, though I've tried threading
just about everything else, from a simple subroutine defined in my Form.vb
to a separate class, and even separate DLL's.

I think the problem might just be the layout of the code. Do you actually
stop the thread anywhere? Just because you close the second form, it
doesn't stop the Do...Loop that is currently running on that thread. It's
untested, but I think you'll need to declare the thread object outside the
function, then call a thread.abort followed by a thread.join. Don't forget
you can also check the status of your thread by calling Thread.ThreadState.

Hope this helps,
-Jason
 
N

Nick

Hi There,

So let me get this right, the thread is running in...

Public Sub AskIfCancel()
Do
Application.DoEvents()
System.Threading.Thread.Sleep(500)
Loop
End Sub

I would suggest using a try catch and aborting the thread when needed,

Public Sub AskIfCancel()
Try
While True
'Your loop code in here
End While
Catch ex as ThreadAbortException
'//The thread was aborted.
End Try
End Sub

'//elsewhere
e.abort << This will cause the abort exception to be raised in
the thread, which usually gracefully exits it.

Nick.
 
C

Cor Ligthert

Just me,

What is the purpose from this extra workerthread. I get the idea that you
try to take over the normal handling from the OS which throws events when
something happened. That needs no extra thread.

The OS is working (semi) parallel on your program

Or do I see something wrong?

Cor
 
C

Crouchie1998

From what I can see, you have mCancelled as a local variable declared in the
function, but when you call UserCancelled it will error or not compile
because mCancelled is not declared in UserCancelled. So, declared that in
the declaration section instead.

Also, your ask if cancelled is an infinitive loop because there is no way
out of it. You should do this:

Private Sub AskIfCancelled()

Do

Thread.Sleep...

While mCancelled = True

End Sub

or even better change it to a boolean function

You are raising an event in your code, but what are you doing with it?
Unspecificed

Any chance of posting more code soI/we can see what is actually going on in
your code? If you use this forum with Outlook/Outlook Express then you can
zip your project & attach it to the post.

Awaiting your response
 
J

Just Me

Thanks a lot
Actually I did get that from the book I was using but minimized the code I
presented, but thanks a lot for replying.
 
J

Just Me

Cor, Please see my latest post for the reason.
But I just learned a word : workerthread
That I could have used in that reply!

Thanks
 
J

Just Me

I did post more and I hope better code.
But to answer this: the way out of the loop is the main thread will abort
this (code not shown) but I think it makes sense to end it as soon as it's
need end (as you did).
Two questions,
1) is it common to use: While mCancelled = True
rather then While mCancelled
which I think does the same?

2)I do t.Abort()
In case the workerthread has been terminated as you suggest do I have to
first check something like
If t Is Nothing


Thanks
 
J

Just Me

Did some looking so I revised below
Just Me said:
I did post more and I hope better code.
But to answer this: the way out of the loop is the main thread will abort
this (code not shown) but I think it makes sense to end it as soon as it's
need end (as you did).
Two questions,
1) is it common to use: While mCancelled = True
rather then While mCancelled
which I think does the same?

2)I do t.Abort()
In case the workerthread has been terminated as you suggest do I have to
first check something like

If t.IsAlive then t.Abort

Or is simply
t.Abort enough?
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top