StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>There's no conceivable reason to alter the locking in this code snippet. It does however suffer from rather nasty design flaws:</p> <ul> <li><p>the wait loop inside DoWork is a <em>hot wait loop</em>. It burns 100% core, accomplishing nothing 99.99999% of the time. That's in general very unfriendly to the machine you run this code on. It will also cause your program to be unresponsive to added items, even though you burn lots of cpu cycles on trying to detect that. The operating system's thread scheduler will keep you in the dog house for a while after you burned your quantum.</p></li> <li><p>the <em>pendingOrders</em> variable is used as a synchronization object but is just a simple <em>bool</em> variable. Lots of things go wrong when you do that. For one, there are good odds that your code never sees the variable being set to true when you run the Release build of your code. A problem in 32-bit code, such a variable must be declared <em>volatile</em>. It is also inefficient, it can take a while before the thread can observe the assigned value.</p></li> <li><p>the use of lstOrderToProcess.Any() is inefficient. No point in removing just the element at index 0 when you in effect will empty the entire list anyway.</p></li> <li><p>the consumer runs on a BackgroundWorker. Which uses a thread-pool thread to implement the worker. TP threads should in general not run for more than about half a second. Your thread however runs forever, severe impeding the thread-pool scheduler's job to get pending tp thread requests executed. This negatively affects the responsiveness of your entire app.</p></li> </ul> <p>Get ahead by using a regular Thread to run the consumer. And use a better design for the list, you want a <em>Blocking Queue</em>. You can get one that runs on old .NET versions by using the code from the threading masters, Joe Duffy's <a href="http://msdn.microsoft.com/en-us/magazine/cc163427.aspx#S4" rel="nofollow">sample implementation</a> solves your hot wait-loop problem. I'll repost it here, tweaked to not wait for the consumer:</p> <pre><code>public class BlockingQueue<T> { private Queue<Cell<T>> m_queue = new Queue<Cell<T>>(); public void Enqueue(T obj) { Cell<T> c = new Cell<T>(obj); lock (m_queue) { m_queue.Enqueue(c); Monitor.Pulse(m_queue); } } public T Dequeue() { Cell<T> c; lock (m_queue) { while (m_queue.Count == 0) Monitor.Wait(m_queue); c = m_queue.Dequeue(); } return c.m_obj; } } class Cell<T> { internal T m_obj; internal Cell(T obj) { m_obj = obj; } } </code></pre>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload