in reply to Re^8: Segfault on second (identical) call to a sub
in thread Segfault on second (identical) call to a sub

Well ... there may well be something not right with perl - but afaict there is something not right with GD-2.30, either. The following script outputs "11037 11037 ERROR 1", but surely $raw and $raw2 should be equivalent. (Note that I'm looping thru the loop only once .... which means I'm not really looping at all :-) 
use strict;
use warnings;
use GD;

for(1 .. 1) {
   x(100, 100);
   print $_,  " ";
   }

sub x{
    my $im = GD::Image->new( @_);
    my $raw = $im->gd;
    my $im2 = GD::Image->newFromGdData( $raw ) or die "$!, $^E";
    my $raw2 = $im2->gd;
#    my $len = length($raw);
    if($raw ne $raw2) {print length($raw), " ", length($raw2), " ERROR
+ "}
}

[download]
If I then remove the '#' so that the length of $raw is evaluated, the script then outputs "1 " - which is as I would expect.

And when I further change the script so that it loops 10 times, I get the "Free to wrong pool..." error immediately (irrespective of whether the length of $raw is evaluated or not). Contrast that behaviour with the following Inline::C script that does (I believe) precisely the same thing - but manages to interface with the GD library flawlessly. (You'll probably need to amend the 'LIBS' and 'INC' config info.) 
use warnings;
package my_gd;

use Inline C => Config =>
    BUILD_NOISY => 1,
    LIBS => '-LD:/gd/gdwin32 -lbgd',
    INC => '-ID:/gd/gdwin32/include';

use Inline C => <<'EOC';
#include <stdio.h>
#include <gd.h>

SV * new(int x, int y) {
     gdImagePtr * image;
     SV * obj_ref, *obj;

     New(1, image, 1, gdImagePtr);
     if(image == NULL)croak("Failed to allocate memory in new()");
     obj_ref = newSViv(0);
     obj = newSVrv(obj_ref, "my_gd");

     *image = gdImageCreate(x, y);

     sv_setiv(obj, (IV)image);
     SvREADONLY_on(obj);
     return obj_ref;
}

SV * gdgdImageCreateFromGdPtr(SV * data, int len) {
     gdImagePtr * image;
     SV * obj_ref, *obj;

     New(1, image, 1, gdImagePtr);
     if(image == NULL)croak("Failed to allocate memory in gdgdImageCre
+ateFromGdPtr()");
     obj_ref = newSViv(0);
     obj = newSVrv(obj_ref, "my_gd");

     *image = gdImageCreateFromGdPtr(len, SvPV(data, len));

     sv_setiv(obj, (IV)image);
     SvREADONLY_on(obj);
     return obj_ref;
}

SV * gdgd(SV * image) {
     void * data;
     int size;
     SV * t;

     data = gdImageGdPtr(*((gdImagePtr *)SvIV(SvRV(image))), &size);
     t = newSVpv((char*) data, size);
     gdFree(data);
     return t;
}

void DESTROY(SV * image) {
     printf("Destroying ");
     Safefree((gdImagePtr *)SvIV(SvRV(image)));
}

EOC
$| = 1;
for(1 .. 10) {
   x(100, 100);
   print $_, " ";
   }
sub x{
    my $im = new(@_);
    my $raw = gdgd($im);
    my $im2 = gdgdImageCreateFromGdPtr($raw, length($raw));
    my $raw2 = gdgd($im2);
#    print length($raw), " ";
    if($raw ne $raw2) {print "ERROR"}    
}

[download]
Note that DESTROY() prints "Destroying " - just so you can see that/when it's being called. It's probably less confusing if you comment that out.

I'm going to stop short of saying that there is something wrong with the GD-2.30 source - I'm not all that familiar with either the GD library or the perl module, and I might be doing something (embarrassingly) stupid. But I feel that it's possible to write the perl interface to the GD library in such a way that the errors we're seeing with the GD module do not occur.

Cheers,
Rob

Replies are listed 'Best First'.
Re^10: Segfault on second (identical) call to a sub
by BrowserUk (Patriarch) on Feb 07, 2006 at 04:53 UTC

    First. Many thanks for having continued to pursue this. Above and beyond.

    but afaict there is something not right with GD-2.30, either

    I was reaching a similar conclusion myself--and it doesn't appear to be limited to 2.30 (which I have yet have pass it's own testsuite so I haven't installed it).

    Having reverted to 5.8.6, I find that GD 2.28 also produces this error there, though I have to exercise it harder, (loop more times), before it occurs. If I hadn't seen the error several times with other, non-GD, non-threads code since I upgraded to 5.8.7, I would've been more inclined to point the finger at GD despite the authorship.

    I am a neophyte as far as XS is concerned. However, I think I may have cracked it? 

    GD::Image
gdnewFromGdData(packname="GD::Image", imageData)
    char *  packname
    SV *    imageData
    PROTOTYPE: $$
        PREINIT:
          char*    data;
          STRLEN   len;
    CODE:
    data = SvPV(imageData,len);
    RETVAL = (GD__Image) gdImageCreateFromGdPtr(len,(void*) data);
//        safefree(data); /// HERE!
    OUTPUT:
    RETVAL

    [download]

    See HERE! above. I made this change and (on very first blush), it appears to fix the problem with my OP code and the original from which it is derived.

    If I understand the above correctly, data is just a pointer to a part of an SV that I pass in, so the library should not be freeing it? It should leave it intact until the SV get gc'd in the normal way? Does that make sense to you?

    If correct, then the newFromGD2Data() call is similarly affected, but not any of the others (png/jpeg/WBMP) as they do not free the data passed in.

    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
    "Science is about questioning the status quo. Questioning authority".
    In the absence of evidence, opinion is indistinguishable from prejudice.
[reply]
[d/l]
[select]
      I'm no expert on XS files either, which is why I usually let Inline::C handle that side of things ... but I don't think 'data' should be safefree()'d ... and I wish you had pointed it out sooner :-)

      I got sidetracked into thinking that there must be something bodgey in the way the GD::Image objects were being created.

      You could probably go a little further and not even declare and assign a value to 'data'. I think one could just as well write 
      RETVAL = (GD__Image) gdImageCreateFromGdPtr(len,(void*)SvPV(imageData,
+ len));

      [download]
      but that's only a minor point.
      It's unclear to me how 'len' gets assigned the appropriate value for when it's needed ... but it seems to happen ok.

      You should probably go to http://rt.cpan.org/Public/Dist/Display.html?Name=GD and report the bug there - or at least let the author know.

      Well spotted !!

      Cheers,
      Rob
[reply]
[d/l]
        and I wish you had pointed it out sooner :-)

        I wish I'd noticed it earlier :) It was only by comparing your Inline C to the XS that it stood out. Previously I'd just checked that allocs and frees were being done with XS macros and not crt functions.

        Thanks again for your help.

        It's unclear to me how 'len' gets assigned the appropriate value for when it's needed

        I wondered that too, but if you unwind the SvPV() macro, you eventually realise that the len parameter passed to it is not used to indicate the length of anything, but rather is used to retrieve the length of the data held in the SV. This is done using the SvCUR() macro. It takes a bit of unwinding and could probably bear a mention in perlapi.

        I've updated GD.xs 2.30 locally with this and got rid of a few unused var warnings which allowed it to compile clean and pass it's test suit. I then thrashed it for a while calling the OP sub and another like it that used GD2 data in a loop and it seems fine.

        I've generated a patch against 2.30 and sent it directly to L.Stein along with a warning that I don't really know what I am doing with this stuff.

        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.
[reply]

In Section Seekers of Perl Wisdom