Using a thread to populate a class member

[aine_canby]

I have a class which has a member which takes a long time to populate.
Some of this member is serialised with an xml file, and some is
established through a set of commands issued on the system. For this
reason I have decided to populate this member using a thread.

class MyClass
{
Member member

Thread populateMemberThread;

bool isMemberPopulated;

PopulateMember()
{
isMemberPopulated = false;

populateMemberThread = new
System.Threading.Thread(PopulateMemberThread);
populateMemberThread.Start();
}

PopulateMemberThread()
{
member = new Member();

XmlSerializer xmlSerializer = ...
...

isMemberPopulated = true;
}

StopPopulateMember()
{
if (populateMemberThread.ThreadState !=
System.Threading.ThreadState.Stopped)
populateMemberThread.Abort();

}
}

The data associated with this member is displayed in a tree view on
the MainForm when member is fully populated. This is done as follows -

populateMemberTreeViewThread = new
System.Threading.Thread(PopulateMemberTreeViewThread);
populateMemberTreeViewThread.Start();

void PopulateMemberTreeViewThread()
{
while (!myClassObject.IsMemberPopulated )
{
Thread.Sleep(500);
}

this.Invoke(new
PopulateMemberTreeViewDelegate(PopulateMemberTreeView));
}

// Form Main's OnClosing
private void OnClosing(object sender, EventArgs ev)
{
if (populateMemberTreeViewThread.ThreadState !=
System.Threading.ThreadState.Stopped)
populateMemberTreeViewThread.Abort();

myClass.StopPopulateMemberThread();
}

So, my question is what improvements can you suggest regarding my code
above.

One issue I can see is if member was accessed before it was finished
being populated. Should I therefore lock member when I populate it?

Thanks,

Aine.

 

[Marc Gravell]

well, the while loop on a non-volatile bool is not guaranteed to ever
exit. And a sleep loop would almost certainly be better achieved with
a Monitor.Wait, which could solve both problems at once. But I'm not
sure how you intend to use this code? it looks like you need 3
threads... one for the UI, one doing the populating, one one waiting
for IsMemberPopulated.

Would it not be simpler to simply use a background thread to run
simple code that builds an object in the usual way, before calling
back onto the UI thread? Allowing for cancellation is a bit tricky,
but Thread.Abort() is a very unsatisfying way of implementing this.
But the below is far less complex; you could still use a Thread
instead of ThreadPool if you absolutely must have Abort(), but...

class MyForm : Form
{
protected override void OnLoad(EventArgs e)
{
base.OnLoad(e);
ThreadPool.QueueUserWorkItem(LoadObject);
}
void LoadObject(object state)
{
// regular slow creation code of newObj...
object newObj = null; // just to compile
this.Invoke(new WaitCallback(ObjectLoaded), newObj);
// (may need to catch error if form disposed while populating)
}
void ObjectLoaded(object value)
{
// update UI; we got out object!
}
}

Marc

 

[Nicholas Paldino [.NET/C# MVP]]

Aine,

Well, using Abort is always a bad idea. You should have another flag,
"aborted" or something like that which you would check at various times in
your PopulateMemberThread method. If the flag is true, then you would just
exit the thread gracefully. You would set the flag, of course, in your
StopPopulateMember method.

Of course, as you indicated, you will need to synchronize access to the
isMemberPopulated field, as well as the "aborted" field (if you decide to
use that). Also, you will have to synchronize access to the actual data as
well, since you are populating it on one thread, and accessing it on
another.

 

[Ben Voigt [C++ MVP]]

I have a class which has a member which takes a long time to populate.
Some of this member is serialised with an xml file, and some is
established through a set of commands issued on the system. For this
reason I have decided to populate this member using a thread.

You don't need so many threads. Just use:

void PopulateMemberTreeViewThread()
{
myClassObject.PopulateMember();
Invoke(new PopulateMemberTreeViewDelegate(PopulateMemberTreeView));
}

Of course you can also use BeginInvoke and the completion callback instead
of managing the threading yourself, but then you couldn't cancel so easily.
You should try to give the thread a chance to stop cleanly before aborting
it. If you are using a flag from multiple threads instead of a heavyweight
lock object, you should at least make the flag volatile.