On Thu, 4 Aug 2022 at 01:58, Mark Murawski <markm-li...@intellasoft.net>
wrote:
> I'm still not getting something... if I want to fix the code-as-is and do
> this:
>
> FNsv = get_sv("main::_FN", GV_ADD);
> if (!FNsv)
> ereport(ERROR,
> (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
> errmsg("couldn't fetch $_FN")));
>
> save_item(FNsv); /* local $_FN */
>
I dont get the sequence here. You take the old value of $main::_FN and then
you localize it after you fetch it? That seems weird.
>
> hv = newHV(); // create new hash
> hv_store_string(hv, "name", cstr2sv(desc->proname));
>
Really you shouldnt do this until you have safely managed the refcounts of
all your newly created objects so that if this die's nothing leaks.
>
> svFN = newRV_noinc((SV *) hv); // reference to the new hash
> sv_setsv(FNsv, svFN);
>
> // dostuff
>
> SvREFCNT_dec_current(svFN);
> SvREFCNT_dec_current((SV *) hv);
>
>
> You're saying that the sv_setsv(FNsv, svFN); creates a second ref... so
> in theory I can unref it and then all else would be equal
> but I get this:
>
> WARNING: Attempt to free unreferenced scalar: SV 0x55d5b1cf6480, Perl
> interpreter: 0x55d5b17226c0
>
Why are you decrementing hv? You dont own hv anymore, it's owned by svFN
and after the sv_setsv() call also FNsv. You shouldnt mess with its
refcount anymore.
HV *hv= newHV(); /* until this is attached to something that will get
cleaned up you need to deal with its refcnt */
svFN = newRV_noinc((SV *) hv); /* now the hv is owned by svFN, we no longer
have to worry about hv, just svFN */
sv_setsv(FNsv, svFN); /* now FNsv is a copy of the ref that is in svFN */
So at this point we have two SV's which reference hv, svFN and FNsv, hv
will have a refcount of 2, svFN should have a refcount of 1.
SvREFCNT_dec(svFN);
This will decrement svFN to 0, which will cause it to be freed, but not
before Perl walks the reference tree decrementing the things the reference
contains, so this will trigger an automatic SvREFCNT_dec() on hv. So after
this line the refcount of hv would be 1, and it would be owned by FNsv only.
Btw, if you liberally added sv_dump(svFN); sv_dump(hv); and sv_dump(FNsv);
to your code you would see what it is happening here. It will show you the
refcounts of each object.
> Also.. something I didn't follow was this:
> "or even better, simply dont use it. You dont need it, you can simply turn
> FNsv into an RV, and then set its RV field appropriately, the leak will go
> away and the code will be more efficient. "
>
> How do you turn an SV into an RV without creating this extra reference..
> Aren't I doing this already, with:
> svFN = newRV_noinc((SV *) hv);
>
No. That creates a new SV which is a reference to a hv.
Your code does this basically:
my %hash;
my $ref= \%hash;
$main::_FN= $ref;
Obviously in perl we can write:
my %hash;
$main::_FN= \%hash;
And in XS we can do the same thing. Unfortunately there isn't a utility sub
to do this currently, it has been on my TODO list to add one for some time
but lack of round tuits and all that.
You want code something like this:
sv_clear(FNsv); /* undef the sv */
sv_upgrade(FNsv,SVt_RV);
SvRV_set(FNsv, (SV*)hv);
SvROK_on(FNsv);
and you are done. It's possible that the sv_upgrade() is unnecessary as I
believe every SV is big enough to handle SVr_RV, try with and without,
keeping will make your code more backwards compatible to really old
versions of perl. But in anything modern an RV is a bodyless SV, and thus
every SV can turn into one without allocating a new body and the sv_upgrade
should be superfluous.
Again, make liberal use of sv_dump() it is the XS version of Data::Dumper
more or less.
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
