Herb Sutter (http://www.gotw.ca/) is a leading authority and trainer on C++ software development. He chairs the ISO C++ Standards committee and is a Visual C++ architect for Microsoft, where he is responsible for leading the design of C++ language extensions for .NET programming. His two most recent books are Exceptional C++ Style and C++ Coding Standards (available in August and October 2004). Jim Hyslop is a senior software designer for Leitch Technology International. He can be reached at jhyslop ieee.org.
Well, Junior, looks like I get to show you a little something," Bob's voice came from behind me. I rolled my eyes. "What this time, Bob?"
"It's about this class you wrote," he said, grabbing my keyboard and calling up some source code in my editor. I recognized a class that I had checked in a while ago. Bob pointed at a section of the code. Simplified, it looked like this:
void SharedResource::Add( SomeClient* object ) { //... in debug mode, verify that // object isn't already in // memberContainer... memberContainer.insert( object ); } void SharedResource::Release( SomeClient* object ) { //...in debug mode, verify that // object is in memberContainer ... memberContainer.erase( object ); if( memberContainer.empty() ) delete this; }
"Oh that," I said. "Yeah, that's kind of cool. We thought we'd follow a sort of COMmunist-like approach on this one. The idea is that each SharedResource object begins life by getting new'd up by some factory, then each client object that wants to use it attaches itself by calling Add with its own this as the argument to let the shared object know it's being watched and used. Later, when the client object no longer needs to use the SharedResource object, it calls Release with its own this as the argument. Once the object knows everyone's gone, it gets rid of itself automatically. Actually, the clients usually don't write out the Adds and Releases manually. They use this other smart pointer that does the Add and Release automatically and really makes the calling code nice and clean, and also"
"Stop!" Bob's eyes were glazing. "Stop! Stop!"
"Okay," I paused. "Anyway, I guess doing things that way isn't always appropriate, but in this case we thought it was a good idea."
"Uh. Sure. Y'see, it's like this," Bob said, recovering somewhat and slurping at his latte for an infusion of energy. "I read somewhere that copy-on-write objects can't be made thread-safe."
"I don't think that's quite right. They can be made thread-safe, just not without adding some extra overhead. But anyway, Bob, this isn't a COW object," I pointed out.
"Who's having a cow?" Wendy's voice floated over the cubicle wall.
"Not 'cows' as in the things that go moo," I called back, "'COWs' as in objects that use lazy copying."
"Don't interrupt," Bob snapped, his latte sloshing perilously close to the edge of his cup, and threatening to slop on my keyboard. "I know this isn't a COW object, I was just using that as an introduction because I read that somewhere once. See, just like I know in my head that it's not possible to make a copy-on-write object thread-safe, I also"
"Hey, wait a minute," I interrupted again. "You mean you actually read a C++ book?"
"Uh, well, I guess Kerry mentioned something about this COW object copying thing to me," Bob mumbled. "Quit interrupting! Like I was saying, just like COW objects, in my experience I've found that self-deleting objects can't be made thread-safe, either. See, I used your object in a multithreaded program, and kept getting these intermittent crashes. I knew it had to be because you'd gone and done something wrong, and after looking at it, I realized it had to be a race condition where two threads were releasing the same contained object at the same time."
"Huh?" I huhhed. "Rewind, I didn't get that. What was the problem?"
"Two. Threads. Call. Release. That's. A. Double. Delete," Bob explained patiently.
"Did you actually find this race condition?"
He waved it off. "Like I said, I thought about it and realized that's what it had to be. All threading bugs are race conditions, aren't they? Well, mostly. Probably. Anyway," he continued, "I tried adding a mutex to serialize access to the list, but even that didn't help. Here's how it looked." He modified the function slightly:
object ) { Lock<Mutex> containerLock( containerLock_ ); // ... in debug mode, verify that // object is in memberContainer ... memberContainer.erase( object ); if( memberContainer.empty() ) delete this; }
"See, what happened is the first thread takes the lock. While it's holding the lock, the second thread tries to take the lock. No can do, buddy, 'cause the first thread owns the lock. The first thread erases the element, finds the container empty, and destroys itself. When the function ends, it releases the lock. The second thread's got the green light now, so it tries to check that the memberContainer is empty. Since the object's been deleted...well, I don't have to tell you what happens." He sat back, a rather smug look on his face.
"Ah, well," I stalled while thinking, "it does look like you're...ah..."
"The word is 'right,' Junior," Bob leaned in to me. "Self-deleting objects can't be made thread-safe. Admit it."
"Yes, well, what you've said is..." To my relief, I heard a snap behind me. Bob jumped, but fortunately his latte cup was empty.
"Nonsense," the Guru firmly finished my sentence. "This object is not thread-unsafe as it was originally written; rather, it was being abused, and the attempted locking was being done at the wrong level. There is no reason to believe that a self-deleting object cannot, in general, be thread-safe."
"Look, dearie, I just showed Junior here why it isn't thread-safe. I don't see how it can be."
"You never were very good at counting," the Guru said softly. The light bulb came on in my head.
"Ah, Bob," I tried to defuse the tension. "I think I see where she's coming from." Bob made some comment I didn't catch, but I think I heard the words "loonie bin."
"No, seriously, Bob, your scenario can't happen, or at least if it does, it isn't a problem with SharedResource. You said there are two objects, on the same or different threads doesn't matter, that both call Release when the size of the memberContainer is 1. That's just a plain old bug because it means the reference count got out of sync. Two references, but a reference count of only 1, capisce? That means the clients are buggy and someone forgot to call Add. Say, are you using our smart pointers or writing out the Add and Release calls by hand?"
Bob was silent for a moment. Then, in what seemed to me to be a somewhat desperate attempt to recover his position, he said: "But it's...how do you call it?...fragile, that's it. It's fragile. You still ought to have some locking in there. What if you have two threads calling Add on the same SharedResource object? Kerblooie, kid, pieces everywhere, bam! It's not like your call to memberContainer.insert( object ) inside Add is going to react well to that, huh, now is it?"
"Uh, well, I guess you just shouldn't do that."
"In this case, my apprentice, perhaps Bob has a point," the Guru put in. "Consider: In general, it is best to enforce the thread safety outside the class because the code that owns an object and knows it is being used by different threads should be responsible for serializing access to such a shared object. The object itself knows not whether it be shared. That puts the onus on the programmer using this object to provide a pious environment in which the class can live in blessed harmony. But here, it could be that the normal use of SharedResource is to be used by clients on different threads. If so, might it not be a valid design decision for SharedResource to serialize access to itself?"
"I guess..." I said slowly.
"All right, already," Bob snapped. "So which way are we gonna do this?"
"This is not a decision that can be made in the abstract," the Guru said, "even though you should prefer to put the locking in the code that manipulates the shared object, rather than inside the shared object. The two of you need to analyze the design to come up with the best approach. The prime question is whether using the same SharedResource from different threads is the exception or the rule. If the exception, then probably the callers should bear responsibility for sharing such objects across threads, else the locking overhead would be incurred for all objects whether shared or not and, therefore, most of the time incurred needlessly. If the rule, the object should probably assume it will be shared across threads and bear the responsibility. Apprentice, show me your writings in half an hour." She reopened her tome and silently glided away.
"Well, Junior," Bob said as he grabbed his latte cup, "guess we better get cracking. See you in the meeting room, right after I top off." I sighed and grabbed a pad of paper, dreading the next half hour.
References
- [1] Sutter, H. More Exceptional C++, Addison-Wesley, 2002.