0

I have a custom collection (a thread-safe ObservableQueue). I implemented the business logic inside the collection class (i.e. dequeue the items one by one and expose them to the outside). This is working fine. To prevent the collection from blocking the thread it is initialised in, the OnservableQueue implements a thread to perform that work. Now I am not perfectly sure of any pitfalls that could occur.

Is it a bad idea to initialise (not start! only initialise) the thread in the constructor? And what would be a good, if not best, practice of terminating the thread? Note, I dont need to know how to terminate a thread, that is working fine, I am rather interested in weather there is something wrong doing it using the disposable pattern or creating a method which would need to get called to terminate the thread. If implementing IDisposable are there any things I have to take in account regarding the collection/queue?

Edit: The thread is actually only pre-initialised to prevent NullReferenceException from being thrown in the Enqueue Method, where it is properly initilised again (the Enqueue Method is supposed to check weather a dequeuing thread is running already and if not to start a new one). Note that whenever all items are dequeued and the thread has done its work it will not be alive any longer either, so any time the queue is empty and a new item is added a new thread will get started to process the queue:

if (!_dequeuingThread.IsAlive)
{
    // start the dequeuing thread
    _dequeuingThread = new Thread(new ThreadStart(StartDequeuing));
    _dequeuingThread.Name = "DeQueueThread";
    _dequeuingThread.Start();
}

The if-statement does need an initialised thread. There are other possible ways of achieving this, but pre-initialising the thread seemed the least bothersome. You see that after checking weather the thread is alive, which it should not when being pre-initialised, it gets initialised again properly.

Sascha Hennig
  • 2,556
  • 1
  • 19
  • 22

2 Answers2

0

I don't see anything wrong with initialising in the constructor, but obviously bare in mind they will be initialised in a different thread than your worker thread.

As for stopping, I generally have a volatile boolean flag that the worker checks to keep running. If your worker thread sleeps at all, then have it wait on an event rather than sleeping, so you can wake it up immediately when stopping it.

GazTheDestroyer
  • 20,722
  • 9
  • 70
  • 103
  • The worker thread is running in a tight loop iterating through the items in the queue dequeing them. In that loop I check the truth value of a volatile bool. And it is iterating the loop or not alive. So where would you actually set the flag? In a method (`StopDequeuing()`) or implement `IDisposable`? – Sascha Hennig Oct 20 '11 at 11:58
  • Personally both. I'd have a StopDequeuing() function that sets the flag and waits for the worker thread to exit, and I'd have a Dispose() implementation that called StopDequeuing(). – GazTheDestroyer Oct 20 '11 at 12:24
0

There seems to be a problem with the fact that the consumer will initialize this collection object by calling its constructor and it would think that the object is initialized (that what the constructor is supposed to do), which is not correct as the initialization is happening on a separate thread created by the constructor. So, basically you need to implement some sort of "Asynchronous API on this object" to initialize this collection such that the consumer calls the initialize method (after creating the object using constructor) and then either by either passing a callback to the initialize method or by registering to an event on the collection object the consumer gets to know that the initialization has been completed.

Ankur
  • 33,367
  • 2
  • 46
  • 72
  • Actually the thread is initialised in the constructor only so that a later call to `Thread.IsAlive` will not throw a `NullReferenceException`. Before the actual thread is _started_ it will get properly initialised again - as this needs to be done if a previous dequeing thread has finished its work (and is not alive any longer) anyway. Apart from that its an thread-safe ObservableQueue implementing the `INotifyCollectionChanged` Interface and a `Dispatcher`, so it initialises OK and perfectly updates whatever it is bound to :) See my edit above for how the thread is really initialised. – Sascha Hennig Oct 20 '11 at 11:32