6

From examples I've seen COM IUnknown::Release() function implementation is something like that:

ULONG Release()
{
    InterlockedDecrement(&m_count);
    if(m_count == 0) {
       delete this;
    }
    return m_count;
}

So, if m_count is 0, so we're deleting "this" object, and returning the ref count. What I don't understand is why it works ?!?!

  1. Deleting the object wouldn't ruin the call stack or is it okay because it is being held by the thread, so it has nothing to do with the object ???

  2. If the object has been deleted, how is it possible that we can return m_count, it should've been deleted. I could have convinced myself that it's okay if after the delete the code would return hard-coded 0, but how come it can return the member ?!?!

Thanks a lot for your help! :-)

sharptooth
  • 167,383
  • 100
  • 513
  • 979
TCS
  • 5,790
  • 5
  • 54
  • 86
  • 1
    +1 You're right - either this is buggy code or there's some more nuanced at work here. I'm curious if someone with more experience with COM can answer this, but my gutshot is that this is flat-out wrong. – templatetypedef Feb 01 '11 at 18:29

2 Answers2

16

That code is bogus. One can never trust m_count after the decrement. The correct code is always like this:

ULONG Release()
{
     ULONG count = InterlockedDecrement(&m_count);
     if(count == 0){ delete this; }
     return count;
}
Remus Rusanu
  • 288,378
  • 40
  • 442
  • 569
  • Did you mean "**after** the *delete*"? – John Dibling Feb 01 '11 at 18:33
  • 2
    No, he meant after the decrement. If multiple threads are accessing this, it is possible that another thread also calls Release, making the value of the member variable negative if it is done in between the decrement and the conditional. – Zac Howland Feb 01 '11 at 18:36
  • 6
    No, I really mean after the `Decrement`. The issue of dereferencing `this` after `delete` is obvious and doesn't need repeat. But there is a more subtle issue of concurrency. Your decrement might bring the count to, say, 2. By the time you touch m_count again, since you already released your count, *other* threads might have have reached 0 and released the object, and the slot might have been even reallocated to something else. So don't touch m_count after the Decrement *hic sunt leones*. – Remus Rusanu Feb 01 '11 at 18:37
  • OP wasn't asking about multithreading, though. The code mentioned it (via Interlocked) but I gather that OP simply copy/pasted this code from someone else's work. – John Dibling Feb 01 '11 at 18:50
  • @John Dibling: in my book 'Interlocked' automatically means multithreading, whether the OP calls it out or not. Going over the fine print and asking if this COM object lives in an STA, MTA or Free apartment I think would be beyond the scope here. – Remus Rusanu Feb 01 '11 at 19:00
  • Yeah, I did copy it, I was trying to understand how to it correctly and keeping it thread-safe. So I saw this code, and I got confused. I must stress that I've seen other examples without InterlockedDecrement that still use m_count after the deletion. And that leads me to a question (just to be sure that I got it) - is it even possible to use m_count after deletion or is it total garbage at that point ? and Thanks again! :-) – TCS Feb 01 '11 at 19:06
  • 1
    It is possible, but is unsafe. The memory is not destroyed on release, so it *may* contain the previous values and 'look' correct. But is up to chance. By the time you look at it, another thread might have allocated exactly the same spot and changed the content. Even your own thread can re-allocate the same spot, if you call anything that allocates memory. – Remus Rusanu Feb 01 '11 at 20:14
1

What you observe is undefined behavior. The call stack is not changed by delete this; and delete this by itself is always safe but renders this pointer invalid which means you can't dereference it anymore.

There're two possible explanations of what you observe. Either the implementation in question just doesn't dereference this pointer to obtain m_count when returning from the function - it has it loaded onto a register and just uses that value and so this is not dereferenced and you don't observe any problem or when delete finishes the memory occupied by the object is still mapped into the process address space and remains technically accessible and so dereferencing this succeeds and m_count is read successfully. I suppose the latter is more likely.

Whatever the explanation is that undefined behavior, you can't rely on that, use what user Remus Rusanu suggests in his answer.

Community
  • 1
  • 1
sharptooth
  • 167,383
  • 100
  • 513
  • 979