Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
Alex Hunsaker escribió: > On Wed, Mar 5, 2014 at 12:55 PM, Alex Hunsaker wrote: > > On Wed, Mar 5, 2014 at 12:22 PM, Alvaro Herrera > > wrote: > > > >> Can I bug you into verifying what supported releases need this patch, > >> and to which does it backpatch cleanly? And if there's any to which it > >> doesn't, can I further bug you into providing one that does? > > > > Sure! Not bugging at all. I'll dig into this in a few hours. > > This will apply cleanly all the way to REL9_2_STABLE. It applies (with > fuzz, but cleanly to REL9_1). REL9_0 does this completely differently > and so does not have this leak. Excellent, thanks. I verified all these assertions and then pushed. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
Hi! On Thu, Mar 6, 2014 at 6:59 AM, Alex Hunsaker wrote: . . . > This will apply cleanly all the way to REL9_2_STABLE. It applies (with > fuzz, but cleanly to REL9_1). REL9_0 does this completely differently > and so does not have this leak. > Looks like patch still not pushed to repo. -- Sergey Burladyan
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
On Wed, Mar 5, 2014 at 12:55 PM, Alex Hunsaker wrote: > On Wed, Mar 5, 2014 at 12:22 PM, Alvaro Herrera > wrote: > >> Can I bug you into verifying what supported releases need this patch, >> and to which does it backpatch cleanly? And if there's any to which it >> doesn't, can I further bug you into providing one that does? > > Sure! Not bugging at all. I'll dig into this in a few hours. This will apply cleanly all the way to REL9_2_STABLE. It applies (with fuzz, but cleanly to REL9_1). REL9_0 does this completely differently and so does not have this leak. Thanks! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
On Wed, Mar 5, 2014 at 12:22 PM, Alvaro Herrera wrote: > Can I bug you into verifying what supported releases need this patch, > and to which does it backpatch cleanly? And if there's any to which it > doesn't, can I further bug you into providing one that does? Sure! Not bugging at all. I'll dig into this in a few hours. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
Alex Hunsaker escribió: > On Tue, Feb 25, 2014 at 6:56 AM, Sergey Burladyan wrote: > > > It looks like I found the problem, Perl use reference count and something > > that > > is called "Mortal" for memory management. As I understand it, mortal is > > free > > after FREETMPS. Plperl call FREETMPS in plperl_call_perl_func() but after > > it, > > plperl ask perl interpreter again for new mortal SV variables, for example, > > in > > hek2cstr from plperl_sv_to_datum, and this new SV is newer freed. > > So I think hek2cstr is the only place we leak (its the only place I > can see that allocates a mortal sv without being wrapped in > ENTER/SAVETMPS/FREETMPS/LEAVE). > > Does the attached fix it for you? Can I bug you into verifying what supported releases need this patch, and to which does it backpatch cleanly? And if there's any to which it doesn't, can I further bug you into providing one that does? Thanks, -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
Alex Hunsaker writes: > On Tue, Feb 25, 2014 at 6:56 AM, Sergey Burladyan wrote: > > > It looks like I found the problem, Perl use reference count and something > > that > > is called "Mortal" for memory management. As I understand it, mortal is > > free > > after FREETMPS. Plperl call FREETMPS in plperl_call_perl_func() but after > > it, > > plperl ask perl interpreter again for new mortal SV variables, for example, > > in > > hek2cstr from plperl_sv_to_datum, and this new SV is newer freed. > > So I think hek2cstr is the only place we leak (its the only place I > can see that allocates a mortal sv without being wrapped in > ENTER/SAVETMPS/FREETMPS/LEAVE). Yeah, I also try to fix only hek2cstr, but failed. > Does the attached fix it for you? Yes, your patch is fix it for me, thank you, Alex! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
On Tue, Feb 25, 2014 at 6:56 AM, Sergey Burladyan wrote: > It looks like I found the problem, Perl use reference count and something that > is called "Mortal" for memory management. As I understand it, mortal is free > after FREETMPS. Plperl call FREETMPS in plperl_call_perl_func() but after it, > plperl ask perl interpreter again for new mortal SV variables, for example, in > hek2cstr from plperl_sv_to_datum, and this new SV is newer freed. So I think hek2cstr is the only place we leak (its the only place I can see that allocates a mortal sv without being wrapped in ENTER/SAVETMPS/FREETMPS/LEAVE). Does the attached fix it for you? diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index 930b9f0..3bc4034 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -304,6 +304,16 @@ static char *setlocale_perl(int category, char *locale); static char * hek2cstr(HE *he) { + char *ret; + SV *sv; + + /* + * HeSVKEY_force will return a temporary mortal SV*, so we need to make + * sure to free it with ENTER/SAVE/FREE/LEAVE + */ + ENTER; + SAVETMPS; + /*- * Unfortunately, while HeUTF8 is true for most things > 256, for values * 128..255 it's not, but perl will treat them as unicode code points if @@ -328,11 +338,17 @@ hek2cstr(HE *he) * right thing *- */ - SV *sv = HeSVKEY_force(he); + sv = HeSVKEY_force(he); if (HeUTF8(he)) SvUTF8_on(sv); - return sv2cstr(sv); + ret = sv2cstr(sv); + + /* free sv */ + FREETMPS; + LEAVE; + + return ret; } /* -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
Hello, All! eshkin...@gmail.com writes: > create function perl_test(IN data text, OUT v1 text, OUT v2 integer, OUT v3 > integer, OUT v4 json, OUT v5 json) > returns record as > $BODY$ > > use strict; > use warnings; > > my $res->{'v1'} = 'content'; > > return $res; > > $BODY$ > language plperlu volatile strict; > test case: > select count(perl_test('')) from generate_series(1, 100); It looks like I found the problem, Perl use reference count and something that is called "Mortal" for memory management. As I understand it, mortal is free after FREETMPS. Plperl call FREETMPS in plperl_call_perl_func() but after it, plperl ask perl interpreter again for new mortal SV variables, for example, in hek2cstr from plperl_sv_to_datum, and this new SV is newer freed. I experiment with this patch, and it fix memory leak in my case, but patch is incomplete. It does not free on error and fix only plperl_func_handler, plperl_trigger_handler and may be plperl_inline_handler must be also fixed. Patch for REL9_2_STABLE. without patch: PIDVSZ RSS 2503 74112 7740 2503 152928 86860 2503 232208 165836 2503 310732 244508 2503 389264 323032 with patch: PIDVSZ RSS 4322 74112 7740 4322 74380 8340 4322 74380 8340 4322 74380 8340 4322 74380 8340 diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index 49d50c4..9c9874d 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -2173,6 +2173,9 @@ plperl_func_handler(PG_FUNCTION_ARGS) ReturnSetInfo *rsi; ErrorContextCallback pl_error_context; + ENTER; + SAVETMPS; + if (SPI_connect() != SPI_OK_CONNECT) elog(ERROR, "could not connect to SPI manager"); @@ -2271,6 +2274,9 @@ plperl_func_handler(PG_FUNCTION_ARGS) SvREFCNT_dec(perlret); + FREETMPS; + LEAVE; + return retval; } -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers