Re: ldap: fix resource leak

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: ldap: fix resource leak
Date: 2006-11-06 00:16:14
Message-ID: 26440.1162772174@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Neil Conway <neilc(at)samurai(dot)com> writes:
> On Sat, 2006-11-04 at 23:34 -0500, Tom Lane wrote:
>> Perhaps use a PG_TRY construct?

> At least for the existing code, this doesn't work well: the function
> exits early via ereport(LOG) and then "return STATUS_ERROR;", so AFAICS
> there isn't an easy way to simplify the existing error handling logic
> via PG_TRY.

OK, no biggie.

> Note that this is related to a more general problem: if *any* backend
> function allocates a resource that needs to be manually cleaned up, it
> probably ought to be using a PG_TRY block. Otherwise, the resource will
> be leaked on elog(ERROR). I wouldn't be surprised if various parts of
> the backend neglected to get this right.

For the most part we've tried to see to it that manual cleanup isn't
required, although I agree that ldap_unbind doesn't seem worth having a
tracking mechanism for.

> However, in this particular
> case, I didn't bother doing this, since it didn't seem likely that
> anything we're going to invoke will report errors via elog. One could
> make an argument for doing, for the sake of correctness/paranoia...

In theory one could put
START_CRIT_SECTION();
END_CRIT_SECTION();
around the code, as a form of "Assert(no elog here)". Not sure that
this is actually a net win though, as a PANIC might well be considered a
worse problem than a one-time leak of some LDAP state.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2006-11-06 00:28:48 Re: ldap: fix resource leak
Previous Message Neil Conway 2006-11-05 23:43:05 Re: ldap: fix resource leak

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2006-11-06 00:28:48 Re: ldap: fix resource leak
Previous Message Neil Conway 2006-11-05 23:43:05 Re: ldap: fix resource leak