Re: Potential memory leak in contrib/intarray's g_intbig_compress

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Konstantin Knizhnik <knizhnik(at)garret(dot)ru>
Subject: Re: Potential memory leak in contrib/intarray's g_intbig_compress
Date: 2023-07-13 15:20:53
Message-ID: 2881020.1689261653@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> writes:
> My collegue Konstantin (cc-ed) noticed that the GiST code of intarray
> may leak memory in certain index operations:

Can you demonstrate an actual problem here, that is a query-lifespan leak?

IMO, the coding rule in the GiST and GIN AMs is that the AM code is
responsible for running all opclass-supplied functions in suitably
short-lived memory contexts, so that leaks within those functions
don't cause problems. This is different from btree which requires
comparison functions to not leak. The rationale for having different
conventions is that btree comparison functions are typically simple
enough to be able to deal with such a restriction, whereas GiST
and GIN opclasses are often complex critters for which it'd be too
bug-prone to insist on leakproofness. So it seems worth the cost
to make the AM itself set up a throwaway memory context.

(I don't recall offhand about which rule the other AMs use.
I'm also not sure where or if this choice is documented.)

In the case at hand, I think the pfree is useless and was installed
by somebody who had mis-extrapolated from btree rules. So what
I'm thinking would be sufficient is to drop it altogether:

- if (in != DatumGetArrayTypeP(entry->key))
- pfree(in);

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-07-13 15:30:03 Re: MERGE ... RETURNING
Previous Message Tom Lane 2023-07-13 15:01:37 Re: psql: Add role's membership options to the \du+ command