[Kde-bindings] Problem condition in Qyoto MethodCall

[Phil Thompson]

On Monday 12 February 2007 5:13 pm, Arno Rehn wrote:
> Am Montag, 12. Februar 2007 schrieb Richard Dale:
> > On Monday 12 February 2007, Arno Rehn wrote:
> > > > [ ... cut ... ]
> > >
> > > Well, I've hacked a lot around in qyoto.cpp and changed the way we
> > > check if it's a contained instance or not: I've added a field to
> > > smokeqyoto_object called 'contained' and it is set up when the
> > > constructor was just called. This was necessary because sometimes the
> > > instance was already destroyed and we still wanted to get the parent of
> > > it. The new field is updated when the parent of the widget is changed
> > > because it is added to a layout, too. It works better now and up to t10
> > > every tutorial works well. But with t11 it keeps crashing and I can't
> > > figure out why it does so. Since I've created quite a mess, I don't
> > > want to check in what I have. I've attached a patch with all the
> > > changes, if you could take a look at it and tell me what you think...
> >
> > I'm not sure if this is the best way to do it, as 'contained' seems to
> > duplicate the function of 'allocated'. If an instance is 'owned' by
> > another instance we can just set 'allocated' to false, and the Qyoto
> > runtime won't delete it when the corresponding C# instance is garbage
> > collected.
>
> Yes, didn't think about that, but it's right. Setting allocated to false
> would be better.
> Do I see that right: If allocated has to be true for a destructor to be
> called, we only want to call destructors for instances which have been
> created by a C# constructor? Then we'd just have to check if it's a
> destructor and if getPointerObject returns something other than 0.

I don't think so. Whether an instance is currently 'contained' has little to 
do with whether it was created by a C# ctor. An instance can be contained or 
not contained at different times depending on how it is used. You only call 
the dtor if it is not contained. You must correctly maintain the state of 
whether an instance is currently contained. If you don't you will either get 
crashes (when there are multiple calls to the dtor) or memory leaks (when 
there are no calls to the dtor). This is why automated bindings generators 
only get you so far. If you want rock-solid bindings you have to look at 
every API call yourself.

> > What problem does it solve that the current scheme with the
> > IsContainedInstance() function doesn't do?
>
> The problem is the following: We have a QLabel which is a child to a
> QWidget, for example. Now the QWidget is destroyed and with it the QLabel
> is gone. Nevertheless the Qyoto runtime wants to call the destructor for
> the QLabel as it still 'thinks' the QLabel exists. To check whether QLabel
> is a child to something and whether to call the destructor we call
> IsContainedInstance() which checks for any parents. Thus it will cause an
> error when trying to call QLabel->parent() as the QLabel is already
> destroyed. Therefore it's better to set this property up when the instance
> is created and when the parent is changed.
>
> > I don't think there's any hurry to solve the problem straight away, and
> > it's best to keep experimenting like you're doing, but not actually
> > commiting anything.
>
> Yes, that's what I thought, too.
>
> > If we use the 'Transfer' data in the PyQt sip files we would need to
> > change ownership when a method, or argument within a method was marked
> > with 'Transfer'. So that data could either go into the smoke library
> > runtime, as it would be useful for every language. Another way would be
> > to add it as C# Attributes, in the 'SmokeMethod' one perhaps.
>
> What exactly is this 'Transfer' data used in PyQt? Haven't heard of it
> before...

The different Transfer annotations are the means by which PyQt knows that an 
instance's 'contained' state has changed. For example...

    QLabel(QWidget *parent /TransferThis/ = 0, Qt::WindowFlags f = 0);

...means that the QLabel instance is contained if parent is non-zero, 
otherwise it is not contained. "Transfer" is short for "transfer 
responsibility for calling the dtor".

Phil