Re: pgsql: Fix breakage from earlier plperl fix.

From: Alex Hunsaker <badalex(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: pgsql-committers(at)postgresql(dot)org, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Fix breakage from earlier plperl fix.
Date: 2012-01-05 23:31:41
Message-ID: CAFaPBrSBz1w-4sLQsWhEZvAyUQ3jUO7sZYPmcVOR6Wia5p72Dw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Thu, Jan 5, 2012 at 16:02, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> Fix breakage from earlier plperl fix.
>
> Apparently the perl garbage collector was a bit too eager, so here
> we control when the new SV is garbage collected.

I know im a little late to the party...

I can't help but think this seems a bit inefficient for the common
case. Would it be worth only copying the sv when its a glob or
readonly? Something like the below? I tested a few more svtypes that
were easy to make (code, regexp) and everything seems peachy.

[ BTW it seems readonly in general is fine, for example elog(ERROR,
1); worked previously as well as elog(ERROR, "1");. Both of those sv's
are readonly. ISTM, at least on my version of perl (5.14.2), globs and
readonly vstrings seem to be the problem children. I think we could
get away with testing if its a glob or vstring. But I don't have time
right now to test all the way back to perl 5.8 and everything
in-between, Ill find it if anyone is interested. ]

--

*** a/src/pl/plperl/plperl_helpers.h
--- b/src/pl/plperl/plperl_helpers.h
***************
*** 47,53 **** sv2cstr(SV *sv)
{
char *val, *res;
STRLEN len;
- SV *nsv;

/*
* get a utf8 encoded char * out of perl. *note* it may not be valid utf8!
--- 47,52 ----
***************
*** 58,65 **** sv2cstr(SV *sv)
* sv before passing it to SvPVutf8(). The copy is garbage collected
* when we're done with it.
*/
! nsv = newSVsv(sv);
! val = SvPVutf8(nsv, len);

/*
* we use perl's length in the event we had an embedded null byte to ensure
--- 57,68 ----
* sv before passing it to SvPVutf8(). The copy is garbage collected
* when we're done with it.
*/
! if(SvTYPE(sv) == SVt_PVGV || SvREADONLY(sv))
! sv = newSVsv(sv);
! else
! SvREFCNT_inc(sv);
!
! val = SvPVutf8(sv, len);

/*
* we use perl's length in the event we had an embedded null byte to ensure
***************
*** 68,74 **** sv2cstr(SV *sv)
res = utf_u2e(val, len);

/* safe now to garbage collect the new SV */
! SvREFCNT_dec(nsv);

return res;
}
--- 71,77 ----
res = utf_u2e(val, len);

/* safe now to garbage collect the new SV */
! SvREFCNT_dec(sv);

return res;
}

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Andrew Dunstan 2012-01-05 23:59:53 Re: pgsql: Fix breakage from earlier plperl fix.
Previous Message Andrew Dunstan 2012-01-05 23:02:48 pgsql: Fix breakage from earlier plperl fix.

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2012-01-05 23:59:53 Re: pgsql: Fix breakage from earlier plperl fix.
Previous Message Andrew Dunstan 2012-01-05 23:02:48 pgsql: Fix breakage from earlier plperl fix.