Friday, October 7, 2011

Supports killing objects

Today I had a really bad "d'oh!" moment... feel free to laugh at me because I did not know better. ;)


You all may use the Supports routine - so did I without further thinking about it - until today. I shamefully have to admit that I should have looked at its code and/or the documentation earlier.

What happened? In my bindings I check the source and target objects for special interfaces (f.i. INotifyPropertyChanged). I changed some code and strange things started happening in form of access violations. So I quickly found out that the Supports call caused a Destroy on the object when it did not support the given interface. Looking at the source revealed why: Supports does not only call GetInterface with the given guid, no it also does it for IUnknown causing _AddRef and _Release calls. And we know how that ends on some refcounted object that has the initial refcount of 0.

Actually there is a comment saying: "NOTE: Calling this overload on a ref-counted object that has REFCOUNT=0 will result in it being freed upon exit from this routine." And the documentation says: "Warning [...] For objects that have a reference count of zero, this will result in the object destruction."

Just another reason why Delphi sometimes is a pain in the ass if you are using object references and interfaces. Some may say: "Just use interfaces!" No, I do not want to write interfaces for simple data objects and other things. But I want to use interfaces for certain aspects (f.i. INotifyPropertyChanged). And most important: I do not want my objects getting destroyed if they DO NOT implement the requested interface.

Actually I have no idea why this was implemented like this long ago. Anyway I wrote my own Supports overload now:

function Supports(const Instance: TObject; const IID: TGUID; out Intf): Boolean;
begin
  Result := Assigned(Instance) and Instance.GetInterface(IID, Intf);
end;

If your instance is assigned for sure you just might call the GetInterface method. I know there might be cases where the other Supports returns true and this one not like when having a customized QueryInterface. But in my cases it should solve my problem of instances getting shot in the head by the Supports call.

Let me know if I missed something about this implementation.

9 comments:

  1. Use managed interfaces. Or use objects. If you have an object reference and you want an interface reference then disable management (i.e. return -1 from _AddRef, _Release).

    ReplyDelete
  2. There was a really long discussion in the Embarcadero forums earlier this year about the possibility of extending Delphi to support a new type of non-refcounted interface (finally!) for exactly this kind of situation. So we might see something like this in the future.

    ReplyDelete
  3. I don't see how a non ref-counted interface can be possible.

    It's not interfaces that are reference counted, it's the implementing object. A non-ref counted interface would simply eliminate addref/release calls on such interfaces.

    But that won't make the difficulties of mixing interfaces and object references any easier, surely ? What if an object implemented both ref counted and NON ref counted interfaces ?

    Then the existing admonition to not mix object references and interfaces simply has to be extended to be "do not mix objects references or non-ref-counted interfaces with ref-counted interaces"

    It surely just makes the existing situation MORE complex, not less !

    The existing situation isn't that difficult to manage (really, it isn't) as long as you properly understand how the interface reference counting mechanism actually works.

    For example, to pull one popular misunderstanding from the air ;) ... the return value of _addRef or _release has nothing whatsoever to do with disabling lifetime management. Lifetime management occurs *within* the _release implementation, *before* it has returned. What it - or addref - returns is actually of no interest and no consequence whatsoever (as far as lifetime management is concerned).

    If anything, if you have disabled lifetime management in _release() then you should arguably always return "1", indicating that yes, at the time of returning from _addRef / _release the object is still very much alive.

    ReplyDelete
  4. You just using your refcounted objects in wrong way.
    If object life is managed by refcounting then you should always hold reference to object's interface instead of object itself, this way you will never get problems like this.

    Ex:
    //it's a simplification, but main idea is shown

    function GetNew: IInterface;
    begin
    Result := TRefCountedObj.Create as IInterface;
    ...
    Supports(Result, IInterface);
    end;

    and never hold this object like:

    var
    Obj: TRefCountedObj;
    begin
    Obj := TRefCountedObj.Create;
    ...
    Supports(Obj, IInterface);
    end;

    ReplyDelete
  5. I disagree. If an object implements ref counting or not should be irrelevant. Because actually I am not using the interface. This is just the question IF the object supports this interface. And even in the case it DOES NOT it gets destroyed. That is the point.

    ReplyDelete
  6. Object can implement interface via overriding of QueryInterface function and delegating implementation to other object (similar to "implements" directive) so checking just by GetInterface may return incorrect result (this why Supports is implemented the way it is, and published here implementation is wrong)

    And again refcounted object should always be used and referenced via interface not by object pointer (this pointer will become invalid and your code will not know it)

    ReplyDelete
  7. I know about the QueryInterface thing - read the last paragraph in my post.

    ReplyDelete
  8. So why are calling this function "Supports" it is not Supports this is wrapped GetInterface.

    And you must know that it is not Supports() killing you object, it is your incorrect usage causes all the problems.
    Supports must cast your object to IUnknown in order to call QueryInterface, and when it is release your object thinks like: "there is none holding me anymore, I can do Destroy now" and this is wrong (not Supports that casts it to interface, this problem will be everywhere where you cast object like that to interface and release it)

    ReplyDelete
  9. I've asked to put that comment in the XMLDoc: http://qc.embarcadero.com/wc/qcmain.aspx?d=118892

    The first thing you should do after object creation is to put it into an IInterface reference. After that start asking for supports. Then it will never kill your object instance.

    ReplyDelete