Replace use malloc() & friend by memory contexts for plperl and pltcl

Michael Paquier <michael.paquier@gmail.com> writes:

Cleanup $subject has been raised a couple of times, like one year ago here:
/messages/by-id/CAB7nPqRxvq+Q66UFzD9wa5UAftYN4WAUADbjXKFrync96kf-VQ@mail.gmail.com
And more recently here while working on the NULL checks for malloc():
/messages/by-id/CAB7nPqR7ozfCqc6C=2+ctCcnuehTZ-vTDQ8MMhY=BQyET0Honw@mail.gmail.com

Attached are a set of patches to replace those memory system calls by
proper memory contexts:
- 0001 does the cleanup work for pltcl
- 0002 does this cleanup for plperl

I looked at 0001. It seemed to me that it wasn't that useful to add a
context unless we did something about actually freeing it; otherwise
we're just increasing the amount of memory leaked after a function
redefinition. However, it proved pretty easy to shoehorn in a refcounting
mechanism like plpgsql has, so I did that and pushed it.

Off to look at 0002 next.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Attached are a set of patches to replace those memory system calls by
proper memory contexts:
- 0001 does the cleanup work for pltcl
- 0002 does this cleanup for plperl

Off to look at 0002 next.

Pushed 0002 as well. The main thing missing there was to use a PG_TRY
block to replace the bulky-and-yet-incomplete assorted invocations of
free_plperl_function.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Sep 1, 2016 at 8:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Attached are a set of patches to replace those memory system calls by
proper memory contexts:
- 0001 does the cleanup work for pltcl
- 0002 does this cleanup for plperl

Off to look at 0002 next.

Pushed 0002 as well. The main thing missing there was to use a PG_TRY
block to replace the bulky-and-yet-incomplete assorted invocations of
free_plperl_function.

Thanks. That's neat!
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 8/31/16 2:57 AM, Michael Paquier wrote:

Hi all,

Cleanup $subject has been raised a couple of times, like one year ago here:
/messages/by-id/CAB7nPqRxvq+Q66UFzD9wa5UAftYN4WAUADbjXKFrync96kf-VQ@mail.gmail.com
And more recently here while working on the NULL checks for malloc():
/messages/by-id/CAB7nPqR7ozfCqc6C=2+ctCcnuehTZ-vTDQ8MMhY=BQyET0Honw@mail.gmail.com

Attached are a set of patches to replace those memory system calls by
proper memory contexts:
- 0001 does the cleanup work for pltcl
- 0002 does this cleanup for plperl

Thoughts?

Seems like a good idea, I'm guessing it slipped through the cracks. Do
you want to add it to the next CF?

I've looked at the pltcl patch, and it looks sane. I did have one
question though...

+	volatile MemoryContext plan_cxt = NULL;
...
+	MemoryContext oldcontext = CurrentMemoryContext;

Why mark one as volatile but not the other? Based on [1] ISTM there's no
need to mark either as volatile?

1: http://www.barrgroup.com/Embedded-Systems/How-To/C-Volatile-Keyword
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532) mobile: 512-569-9461

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Nov 8, 2016 at 7:39 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

On 8/31/16 2:57 AM, Michael Paquier wrote:
Seems like a good idea, I'm guessing it slipped through the cracks. Do you
want to add it to the next CF?

0001 has been pushed as d062245b.

Why mark one as volatile but not the other? Based on [1] ISTM there's no need to mark either as volatile?

plan_cxt is referenced in the PG_TRY block, and then modified in the
PG_CATCH block, so it seems to me that we had better mark it as
volatile to be POSIX-compliant. That's not the case of oldcontext.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers