Hammering on Threading Problems is Tedious Work

bug.gif

Today I'm dealing with a lot of issues regarding performance and threading. It's non-trivial, even for as much of it as I've done, as it's all a balance of safety, speed, and need. It's just plain not easy, but hey… if it were, I wouldn't get paid to do it.

Today I ran across an interesting issue… found this code:

  const calc::results_t & Option::getResults() const
  {
    using namespace boost::detail;
    spinlock::scoped_lock  lock((spinlock &)mResultsMutex);
    return mResults;
  }

Now I know that the intended purpose of this code was to make sure that no one altered the mResults value while a copy was made on the calling stack and returned to the caller. But that's not at all what's happening, is it?

Have a look at the return type - a reference. That means that we're passing back a reference to the mResults value, and not a copy at all. This means that the scoped lock is totally useless in this context. Better to just remove it as there are places in the code that expect to get a reference:

  const calc::results_t & Option::getResults() const
  {
    return mResults;
  }

This is far cleaner, and just as "protected" from multiple thread access - which is to say it isn't.

I'm cleaning up things and trying to track down why I'm getting a deadlock, but it's not at all simple. It's not as easy as looking at the code - if it were, I'd have solved it, but some things are just too well hidden to pop out on a simple inspection. Sometimes, it takes a smoking gun to really point out the problem you have.

So I'm looking for the smoking gun. Hope I find it soon.