Originally authored in August 2014 on LinkedIn, I still generally agree with my comments here, although in the age of lambda’s, every variable may be suspect in multi-threaded software. Thread with care!
My apologies in advance. I couldn’t resist the baiting title. A more academic and precise title for this post might be:
Impact of coding standards and conventions on identifying multithreaded software faults during a code inspection.
Still interested? Proceed forward!
I was recently helping to find the source of an occasional, random, data corruption bug in some legacy software. The behavior of the issue indicated a multithreaded coding flaw: something wasn’t being locked and protected from access by the 10+ threads in this particular software application. The application in question was written in C# targeting WPF. The software was written following Microsoft’s C# coding standard.
During the course of our search for this elusive bug, I became frustrated with the following specific Microsoft recommended coding convention:
Do not use a prefix for field names. For example, do not use g_ or s_ to distinguish static versus non-static fields.
In my past C++ software efforts, which made heavy use of threads, we required the following for class members/fields:
- static class member variables shall be named with an “m_” prefix.
- non-static class member variables shall be named with an “m” prefix.
One could argue that my frustration was entirely due to my past experience. I had lived and breathed and dreamed code in that particular standard for 10 years. That standard made it possible for me to easily read code, and in particular, read code with multithreaded issues in mind. How so?
Before explaining further, we should refresh ourselves on why multithreaded software is such a challenge. I will not re-invent the wheel, so if you need a refresher, or are not familiar with the challenges of multithreaded software, please google it or perhaps review: http://msdn.microsoft.com/en-us/magazine/cc163744.aspx. One quick point: any shared data structures, variables, and objects, of any complexity, must generally be protected in some manner, especially if it is known that such objects will be accessed from multiple thread contexts. Otherwise they may be corrupted and this corruption will likely be random and difficult to reproduce.
Given the challenge, how can the coding convention help? In this particular case, I believe my preferred convention allows the person reviewing the code to immediately understand which variables could be accessed from multiple thread contexts. When reviewing the code, anything with an “m” or “m_” prefix is suspect. Is it protected by a locking mechanism? If not, it should be reviewed more carefully. Local arguments or stack variables will not have a prefix and can generally be ignored when considering multithreaded design issues.
Microsoft’s convention treats all fields the same from a naming convention point of view. There is nothing inherently wrong with this choice. However, while reviewing and inspecting this particular multithreaded code base, I quickly found myself burdened with the additional mental baggage of tracking which variables were local, which were class members, and which were static. Undoubtedly, Microsoft’s Visual Studio IDE made this relatively easy, yet I could not “just read the code.” I found myself frequently needing to reconfirm which variables were local (on the stack) and which were member variables. Anything that adds to a developer’s mental baggage while coding should be reconsidered. My vote: use the prefix, especially if developing multithreaded software.
Ultimately the author of the code found and fixed the issue. A missing lock. We rejoiced and released the software for general internal testing. A good day!