Threading.? Deadlocks.? Race conditions.? These are such seemingly simple items, yet no one seems to have a good answer to how to handle them.? Instead we seem to have 2 categories of developers.? One category is the developers who understand the problem is hard and are careful in their design and approach to avoid the easy errors, and those who want to simply explain away a lack of rigor in their approach as an unavoidable outcome of the nature of the problem.? In my experience, you are far more likely to encounter problems caused by the second group than the first.? Fortunately, the type of errors created by the second group is usually easily identified in a code review and can be fixed by fairly simple means.?
Take, for example, the following code (C#):
List<string> duplicates = null;
System.Threading.ThreadPool.QueueUserWorkItem(
? new System.Threading.WaitCallback(delegate(object target)
? {
??? duplicates = new List<string>();
??? while(/* some condition */)
??? {
????? duplicates.Add(someMethod());
??? }
? }));
foreach(string name in names)
{
? if(isDuplicate(name))
? {
??? duplicates.Add(name);
? }
? //…
}
Can you find the problem?
Of course, this is greatly simplified, and in looking at this code it should be blatantly obvious to the most inexperienced among us that there is a race condition between the threaded work item and the foreach loop.? If the threaded work item does not execute immediately, the foreach will throw out a NullReference exception.? Of course, in this case, we can add an easy fix to the NullReference problem:
List<string> duplicates = null;
System.Threading.ThreadPool.QueueUserWorkItem(
? new System.Threading.WaitCallback(delegate(object target)
? {
??? duplicates = new List<string>();
??? while(/* some condition */)
??? {
????? duplicates.Add(someMethod());
??? }
? }));
while(null == duplicates)
{
? System.Threading.Thread.Sleep(0);
}
foreach(string name in names)
{
? if(isDuplicate(name))
? {
??? duplicates.Add(name);
? }
? //…
}
So all is well, right?
Actually, it sucks.
The immediate risk of failure is averted, but the design is still poor.? It may be that we are required to use this design for some external reason, but on the face of it, it simply sucks.? Why not simply do away with the selective threading entirely and thread the whole method as needed?? By thinking about the problem and the reasons that we wanted to thread, we should be able to come up with several different ways of implementing this that do not suffer from the same drawbacks as the current implementation.? By simply thinking about the problem and what we are trying to accomplish before writing any code, we can eliminate most of these types of errors before they even occur.? But doing that would put us firmly in the camp of the first category of developers, which is much harder than to simply admit defeat.