Re: Memory fault when inserting QTableItem into QTable

--- Richard Dale <[email protected]> wrote:
> On Monday 12 July 2004 03:03, Ashley Winters wrote:
> > --- Richard Dale <[email protected]> wrote:
> > It's explicitly taking responsibility for the destruction of the
> object
> > AWAY from Perl by calling takeItem(), and we need to honor it.
> >
> > So, the patch is a simple one-line fix:
> >
> > -     setAllocated( $_[0], 1 );
> > +     setAllocated( $_[0], was_called_from_perl() );
> >
> > Of course, someone has to write was_called_from_perl(), which needs
> to
> > pass the following test:
> >
> > package test1;
> > use Qt::isa 'Qt::Table';
> >
> > # This does NOT qualify as being called from Perl if it is called
> from
> > # C++
> > sub takeItem { SUPER->takeItem(@_) }
> I think that rather than needing to determine how takeItem() has been
> been 
> called, we need to be able to mark instances as being already deleted
> in 
> QtSmokeBinding::deleted(). The attached patch adds a 'zombie' flag to
> the 
> smokeperl_object struct like this:

That fixes one case, but I was pointing out that, with the current
implementation of takeItem() in PerlQt, it's *always* a bug when it
gets called from C++. Follow this:

# add the item to an object which handles its destruction
$object->addItem(Qt::Item(1));
# at this point, the Qt::Item object has been saved
# into object->{'hidden children'}. refcnt == 1

# Now, we call into C++
$object->doSomething();

// Okay, now we're in C++
void QFoo::doSomething() {
    // ...
    QItem *item = findSpecialItem();   // ahh, here's our Item
    takeItem(item);                    // bug invoked here

    // Uh oh, item has been deleted by Perl!
    sibling->addSpecialItem(item);   // segfault here, or segault later

    // ...
}

It's easy to see why this is a bug:

sub Qt::Foo::takeItem {
    # Since this is being called from the destructor, $_[0] was
    # found in the global object map afterall!
    #
    # There's no named reference to the object anywhere, so,
    # refcnt == 2: $_[0] and this->{'hidden children'}
    delete ${ this()->{"hidden children"} } { sv_to_ptr($_[0]) };

    # Okay, make that refcnt == 1, $_[0] is all we got now

    setAllocated( $_[0], 1 );

    # Uh oh... we just sentenced the last reference to death
}
# Well, the item is dead, and we gracefully avoid directly segfaulting,
# but C++ is now left with a bogus pointer, which, according to TFM,
# they are required to delete somehow after calling takeItem().

In the end, the manual is the enemy. :)

Admittedly, I haven't grepped the Qt/KDE source to find out if any
functions therein ever call takeItem() -- if not, it would absolve
anyone of the /need/ to fix it.

Having a zombie designation is probably useful, but not for silently
sweeping bogus deletions under the rug. Rather, it should provide a
diagnostic. It's never okay for Perl to accidentally reschedule an
object for deletion, since there's no guarantee it's not on the stack
unless we know we created it ourself, in which case it'd be in the
object map! Well, there IS a guarantee in this case, but it's safer to
pretend, lest we discover ourselves in the future scheduling a QEvent
for deletion.

That's why I never put additional checks in there; the removal of the
object from the global map was meant to be sufficient guard against
double-deletion.

Unfortunately, this raises one more issue...

$item = $object->findItemCreatedByQt();
$object->takeItem($item);

$item is scheduled for deletion, but it's not in the global object map.
This contradicts the general rule that Perl is forbidden to delete
objects which aren't "owned" by the object map.

Therefore, takeItem() needs to add the object to the object map if
it's not already in there, just as if takeItem() were a constructor
called from Perl. Ugh.

Ashley Winters


	
		
__________________________________
Do you Yahoo!?
New and Improved Yahoo! Mail - 100MB free storage!
http://promotions.yahoo.com/new_mail