| Date: | September 28, 2005 / year-entry #282 |
| Tags: | code |
| Orig Link: | https://blogs.msdn.microsoft.com/oldnewthing/20050928-10/?p=34013 |
| Comments: | 23 |
| Summary: | As we saw last time, trying to do too much in one's destructor can lead to an object being destroyed twice. The standard way to work around this problem is to set an artificial reference count during destruction. class MyObject : public IUnknown { ... ULONG Release() { LONG cRef = InterlockedDecrement(&m_cRef); if (cRef ==... |
As we saw last time, trying to do too much in one's destructor can lead to an object being destroyed twice. The standard way to work around this problem is to set an artificial reference count during destruction.
class MyObject : public IUnknown
{
...
ULONG Release()
{
LONG cRef = InterlockedDecrement(&m_cRef);
if (cRef == 0) {
m_cRef = DESTRUCTOR_REFCOUNT;
delete this;
}
return cRef;
}
...
private:
}
enum { DESTRUCTOR_REFCOUNT = 42 };
~MyObject()
{
if (m_fNeedSave) Save();
assert(m_cRef == DESTRUCTOR_REFCOUNT);
}
};
If you have a common implementation of
By setting the reference count to an artificial non-zero value,
any
This is really more of a workaround than a rock-solid solution,
because it assumes that no functions called during the destruction
sequence retain a reference to the object beyond the function's
return.
This is in general not something you can assume about COM.
In general, a method is free to call
Exercise:
Why is it safe to perform a simple assignment
|
Comments (23)
Comments are closed. |
As someone pointed out, you might have vtable problems with this solution. But you so say it is a workaround and not a full solution.
Simple assignment works since all references to the object are gone and thus there shouldn’t be any threading issues.
This "m_cRef = DESTRUCTOR_REFCOUNT" is safe simply because if your executing this code nothing else has a reference to the object (in any thread). Only one thread can "Clean up".
If any other thread did have a reference the count wouldn’t have transitioned to zero.
The only caveat is when someone has got a non-referenced counted pointer to the object (intentionally or unintentionally). If this is the case then they deserve everything they get (crashes, corrupt memory, objects disappearing etc)
It’s 2005, no need for the enum hack anymore :)
Um, I would strongly chastise any developer on my team who wrote such code. A reference count of 42 is quite low; when you include callback interfaces, "site" interfaces, etc. 42 is well within the bounds for a reasonable program.
If you must, use a negative value, or at least some huge positive value.
My solution to this has been to mandate that object destructors only deal with freeing memory (breaking reference cycles on subgraphs, etc.) or releasing handles to unmanaged resources. "Semantic" actions, such as saving a file, are forbidden.
"Bytes 2-3 The second word of the file is the TIFF version
number. This number, 42 (2A in hex), is not to be equated
with the current Revision of the TIFF specification….
…The number 42 was chosen for its deep
philosophical significance."
Doing the Save() operation from Release() instead of from the destructor would avoid the vtable problem and allow the object to survive past the "last" Release() call, since you can keep a legitimate reference on the object. However, you can still run into nasty problems with a client function that isn’t reentrant. Often member pointers are released in "mp->Release(); mp = NULL;" order, so if you do something in your Release() that causes recursion into the client function, you’ll hit a double-Release() problem.
I agree with Arlie in that destructors need to be limited to memory housekeeping and not user actions. If the user wants to save/write the settings – let them explicitly do that.
I wouldn’t multipurpose your m_cRef. That’s for certain. What you really need is a simple state variable ("bool m_bShuttingDown"?) or an enum (if you want to get detailed) to check against before doing potentially crashing activities.
@Arlie –
The 42 is not a reference count threshold, it’s just being used as a flag. I suppose Raymond used it because 42 is the answer to all questions =)
Maybe using -1 would work better and be more readable.
Arlie,
Using 42 is completely safe since the assertion will not occur until the ref count has reached zero and the object is being destroyed. At this point any flag value is valid except for 0.
Mandating that destructors only deal with freeing memory and handles and breaking reference cycles is short-sighted. If that is true, then your object must rely on the consumer to perform some critical behavior such as saving before releasing your object which is even worse than the work around presented in this article since no consumer should be required to keep track of your references.
Klaus,
42 is certainly not the answer to ALL questions. It is only the answer to The Ultimate Question of Life, The Universe, and Everything. Now if only we knew the question. :)
Silly question, I’m sure, but why is InterlockedDecrement used?
The only reason I can think of is for multithread safety. But that Release function doesn’t look multithread safe — what if another thread was about to increment m_cRef? Does the AddRef refcount incrementer have a special interlocked check for zero to catch this case?
(Mind you, I don’t know anything about the COM system, I’m just looking at it from the perspective of refcounting.)
of course, if you were using ATL (and implementinf FinalRelease() ) then this would never have been a problem.
// Set refcount to -(LONG_MAX/2) to protect destruction and
// also catch mismatched Release in debug builds
virtual ~CComObject() throw()
{
m_dwRef = -(LONG_MAX/2);
FinalRelease();
#ifdef _ATL_DEBUG_INTERFACES
_AtlDebugInterfacesModule.DeleteNonAddRefThunk(_GetRawUnknown());
#endif
_pAtlModule->Unlock();
}
Daev: Think about how it would be possible for somebody to AddRef() while somebody else was Release()ing to zero.
@kbiel: MAndating that destructiosn deal with trivial things is entirely reasonable. As the ::Release method can deal with what happens when the ref-count hits zero before it destroys the object.
@Arlie: the ‘flag’ value must either be positive or (if negative) very large. A small positive value (eg. 1) is preferred to avoid wrap-around. A small negative value doesn’t work, because a series of increments and decrements could result in another deletion!
Note that the flag value is a lower bound on the possible values the reference count can take (assuming the application increments and decrements correctly – otherwise things go wrong anyway), so 42 is actually fine as a flag value, but definitely -1 or 2147483647 would definitely not be.
Wednesday, September 28, 2005 3:17 PM by oldnewthing
> Daev: Think about how it would be possible
> for somebody to AddRef() while somebody else
> was Release()ing to zero.
I’m not Daev but I’m trying to think about this. It seems to me that the reason why InterlockedDecrement() is needed is: it would be possible for somebody to AddRef() while somebody else was Release()ing to NONZERO. If somebody else was releasing to zero then no one has a valid pointer to this object for the purpose of AddRef(), so maybe the Release() method could try to figure out whether or not it needs to call InterlockedDecrement() at this particular time or not, but there’s no point in trying to figure that out.
One reason the Interlocked functions are needed can been be seen from the disassembly of this simple code:
–m_lRef;
When disassembling the simple prefix decrement above from an executable generated by the VS2002 (x86) C++ compiler you get (something like) the following (assume eax holds the address of m_lRef):
mov ecx,dword ptr [eax]
sub ecx,1
mov dword ptr [eax],ecx
If the kernel (for what ever reason) preempts this thread between the first MOV and SUB instruction and subsequently schedules another thread that completes an AddRef() in it’s entirety, the code above (when scheduled again by the kernel) will be operating on a “stale” value.
This can result in all sorts of strange behavior in a multi-threaded environment. Objects disappearing when threads have “valid” references is just one symptom.
Looking at the disassembly of the “InterlockedDecrement” function (again on an x86 machine) from Kernel32 you can see that the load, decrement and store is done atomically thus alleviating the errors encountered above:
SYM:_InterlockedDecrement@4
0x7C809794: 8B4C2404 MOV ECX,DWORD PTR [ESP+0x4]
0x7C809798: B8FFFFFFFF MOV EAX,0xFFFFFFFF
0x7C80979D: F00FC101 LOCK XADD DWORD PTR [ECX],EAX
0x7C8097A1: 48 DEC EAX
0x7C8097A2: C20400 RET 0x4
Interestingly the COM specification does not guarantee the accuracy of the reference count returned by AddRef() or Release() UNLESS the return value is 0. It’s easy to understand why given the fact that you cannot accurately determine the value of the ref count at any-one point unless you are the thread cleaning up, in which case the object is no longer in a volatile state. (NOTE: This is not the only reason COM doesn’t guarantee these return values)
The Interlocked functions should be used for both AddRef and Release if the code will be used in a multi-threaded project. Even though you only have to know if they hit zero, it still needs to be thread-safe. This is assuming multiple threads can and will be calling AddRef and Release.
The reason you can do a simple assign to cRef is because it is simply that, an assign. An assign is not a read/modify/write operation. It is already atomic. Increment/Decrement are not atomic and so must have a LOCK prefix, or in Windows terms an Interlocked prefix.
Chris: MAndating that destructiosn deal with trivial things is entirely reasonable. As the ::Release method can deal with what happens when the ref-count hits zero before it destroys the object.
You still have to deal with the exact same issues whether you call your save method from ::Release or from the destructor. Beside that fact, not everything is COM.
@Daev
Once the Class Object (essentially a factory for creating COM objects) creates an instance of a COM object, the only way to get another reference to that object is if you already have a reference to it. This means that it is impossible for AddRef() to be called (in a valid program) if the InterlockedDecrement() in Release() results in the reference count being zero.
Some very insightful comments have been made showing why the Interlocked instructions are required in a multithreaded program.
However, my original question was a little different: not why is the InterlockedDecrement necessary, but why is it *sufficient*?
(Raymond’s answer was a very diplomatic way of suggesting I think about how COM works a bit more. I’ve been reasoning from a naive model of refcounted pointers to structs, since I don’t know COM.)
Assuming AddRef is nothing but an InterlockedIncrement instruction, why doesn’t the entire Release function need to be mutex-protected? What if one thread is about to call InterlockedIncrement in AddRef when a second thread gets control, ends up decrementing the object’s refcount to zero, and is then about to call "delete" when the first thread regains control?
PingBack from http://www.antcassidy.com/code/?p=44
PingBack from http://blogs.msdn.com/oldnewthing/archive/2008/11/03/9030246.aspx