[Kde-perl] Memory fault when inserting QTableItem into QTable

[Richard Dale]

On Monday 12 July 2004 03:03, Ashley Winters wrote:
> --- Richard Dale <Richard_Dale at tipitina.demon.co.uk> wrote:
> [snip]
>
> > PerlQt overrides the takeItem() virtual method as part of its garbage
> >
> > collection tracking, and so when the perl version of takeItem() is
> > called the
> > recently deleted instance is found to not have a corresponding perl
> > instance
> > and another is created. Inside the perl takeItem() method the call
> > 'setAllocated( $_[0], 1 );' means that PerlQt owns this newly created
> >
> > instance owns the (already deleted) QTableItem and is responsible for
> >
> > deleting it.
> >
> > sub Qt::Table::takeItem
> > {
> >     package Qt::_internal;
> >     delete ${ this()->{"hidden children"} } { sv_to_ptr($_[0]) };
> >     delete $_[0]->{"has been hidden"};
> >     setAllocated( $_[0], 1 );
> >     no strict 'refs';
> >     $Qt::AutoLoad::AUTOLOAD = 'Qt::Table::takeItem';
> >     my $autoload = " Qt::Table::_UTOLOAD";
> >     dontRecurse();
> >     $autoload->( $_[0] );
> > }
>
> Hmm...
>
> > On garbage collection of the '$_[0]' passed to takeItem(), PerlQt
> > attempts to
> > delete an already deleted instance and there is a crash. Now I
> > understand
> > what's going on I can start to think of a fix..
>
> The takeItem() family of functions need to know who their caller is.
> When being called from C++, they should bypass all garbage-collection
> voodoo. It's either being called from C++ or from Perl, and whichever
> one is calling it beeds to take responsibility for deleting the object
> thereafter.
>
> When called from Perl, there is already a perfectly valid $item
> (otherwise, how did it get passed to takeItem()?), and we are obligated
> to call setAllocated(1) on the object, as well as remove any
> pre-existing 'hidden children' object remaining in the parent object.
> That way, Perl's garbage collection will properly destroy the object
> (just like the programmer was asking in the first place by calling
> takeItem()).
>
> When called from C++, the item being taken must be setAllocated(0), to
> allow the calling function to move or delete the item as it wishes.
> 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(@_) }
>
> package test2;
> use Qt::isa 'Qt::Table';
>
> # How do you propose to pass this one when called from C++?
> sub takeItem { this->Qt::Table::takeItem(@_) }
>
> Okay, test2 counts as being called from Perl even when
> test2::takeItem() is called from C++. Use SUPER for proper behavior.
>
> Of course, all the takeItem() functions need the above one-line patch.
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:

struct smokeperl_object {
    bool allocated;
    bool zombie;
    Smoke *smoke;
    int classId;
    void *ptr;
};

So QtSmokeBinding::deleted() no longer unmaps the instance being deleted, it 
just marks it as a zombie. Later in handlers.cpp, smokeperl_free() always 
unmaps the instance, but only deletes it if it is both allocated and not a 
zombie:

int smokeperl_free(pTHX_ SV *sv, MAGIC *mg) {
    smokeperl_object *o = (smokeperl_object*)mg->mg_ptr;

    const char *className = o->smoke->classes[o->classId].className;
    unmapPointer(o, o->classId, 0);
    if(o->allocated && !o->zombie && o->ptr) {
        if(do_debug & qtdb_gc) fprintf(stderr, "Deleting (%s*)%p\n", 
className, o->ptr);
...

This way the perl takeItem()s don't need any change, because is zombie flag 
will always trump an allocated flag.

-- Richard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: perlqtdispose_0.3.patch
Type: text/x-diff
Size: 3980 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-perl/attachments/20040712/35af4db2/perlqtdispose_0.3.bin