Re: BUG #19411: libpq 16.x exhibits a memory leak when connections are repeatedly created and destroyed

Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> writes:
> On Fri, Oct 30, 2026 at 9:55 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> /* Only try to acquire credentials if GSS delegation isn't disabled. */
>> - if (!pg_GSS_have_cred_cache(&conn->gcred))
>> - conn->gcred = GSS_C_NO_CREDENTIAL;
>> -
>> if (conn->gssdelegation && conn->gssdelegation[0] == '1')
>> - gss_flags |= GSS_C_DELEG_FLAG;
>> + {
>> + if (conn->gcred == GSS_C_NO_CREDENTIAL)
>> + conn->gcred = pg_GSS_get_cred_cache();
>> + if (conn->gcred != GSS_C_NO_CREDENTIAL)
>> + gss_flags |= GSS_C_DELEG_FLAG;
>> + }

> This might be equivalent logic, but I'm not sure.

You're right, it's intentionally not equivalent logic.

> The current code
> always requests delegation if conn->gssdelegation is true (so if
> pg_GSS_get_cred_cache()/gss_acquire_cred() fail, then delegation will
> still be attempted with GSS_C_NO_CREDENTIAL). The new code requests
> delegation only if gss_acquire_cred() succeeds -- if that fails but
> the later GSS_C_NO_CREDENTIAL attempt succeeds for some weird
> GSS-specific reason, shouldn't we still try to delegate as directed?

How can delegation succeed if there's nothing to delegate? Also,
the equivalent stanza in pqsecure_open_gss (fe-secure-gssapi.c,
lines 641-656 in master) behaves this way. I'm much more prepared
to believe that pqsecure_open_gss's behavior is intentional; for
one thing because it matches its comments, which pg_GSS_continue
didn't.

> Are there any delegation cases where we were actually expecting to
> overwrite the previous, valid, credentials?

Doubt it. There is nothing different about what a repeat call
to pg_GSS_get_cred_cache will do, so I'd expect it to fetch
credentials that are exactly like the old ones, modulo perhaps
a slightly newer expiration time.

> I deleted
> pg_GSS_continue's pg_GSS_have_cred_cache() call entirely, and the
> tests still passed on my machine, which makes me worried that maybe
> there's an edge case that was never recorded. (Alternatively, maybe
> the call is completely unnecessary.)

I wondered about that too. I tried running the src/test/kerberos
tests with --enable-coverage, and found that they reach every call
of pg_GSS_get_cred_cache *except* the one in pqsecure_open_gss ---
but that's because we already have credentials there, as evidenced by
the fact that the "gss_flags |= GSS_C_DELEG_FLAG" line is reached.
So we do seem to need pg_GSS_continue's call.

Also, if you look at 6633cfb21, this whole stanza in pg_GSS_continue
was introduced then; before that we never passed credentials nor
GSS_C_DELEG_FLAG to that call of gss_init_sec_context, or the other
one either. So I have to think that's the intended behavior in
non-delegation mode. In any case, it does not seem to make sense
to be passing different parameters to successive calls of
gss_init_sec_context.

But I'm mostly unfamiliar with the details of GSS, so it's possible
I'm all wet here.

regards, tom lane