Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak

2014-03-16 Thread Alvaro Herrera
Alex Hunsaker escribió:
 On Wed, Mar 5, 2014 at 12:55 PM, Alex Hunsaker bada...@gmail.com wrote:
  On Wed, Mar 5, 2014 at 12:22 PM, Alvaro Herrera
  alvhe...@2ndquadrant.com 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

2014-03-15 Thread Sergey Burladyan
Hi!

On Thu, Mar 6, 2014 at 6:59 AM, Alex Hunsaker bada...@gmail.com 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

2014-03-05 Thread Alvaro Herrera
Alex Hunsaker escribió:
 On Tue, Feb 25, 2014 at 6:56 AM, Sergey Burladyan eshkin...@gmail.com 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

2014-03-05 Thread Alex Hunsaker
On Wed, Mar 5, 2014 at 12:22 PM, Alvaro Herrera
alvhe...@2ndquadrant.com 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

2014-03-05 Thread Alex Hunsaker
On Wed, Mar 5, 2014 at 12:55 PM, Alex Hunsaker bada...@gmail.com wrote:
 On Wed, Mar 5, 2014 at 12:22 PM, Alvaro Herrera
 alvhe...@2ndquadrant.com 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

2014-02-26 Thread Sergey Burladyan
Alex Hunsaker bada...@gmail.com writes:

 On Tue, Feb 25, 2014 at 6:56 AM, Sergey Burladyan eshkin...@gmail.com 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

2014-02-25 Thread Sergey Burladyan
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


Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak

2014-02-25 Thread Alex Hunsaker
On Tue, Feb 25, 2014 at 6:56 AM, Sergey Burladyan eshkin...@gmail.com 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