Re: Cache invalidation after authentication (on-the-fly role creation)

On Thu, Jul 5, 2018 at 9:35 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
>> PerformAuthentication(MyProcPort);
>> + AcceptInvalidationMessages();
>> InitializeSessionUserId(username, useroid);
>
> That seems like a *really* ad-hoc place to put it. Why should it be
> there, and not (say) somewhere inside InitializeSessionUserId, or maybe
> (also?) inside PerformAuthentication? Why do the existing call sites for
> AcceptInvalidationMessages --- in particular, the ones associated with
> lock acquisition --- not solve the problem already?

There is no lock acquisition involved here. The sequence is:

1. ClientAuthentication()->hba_getauthmethod()->check_hba()->get_role_oid()
tries to look up user "fred" and doesn't find it (that lookup is used
for group membership checks for hba line matching purposes, and not
finding it here is not fatal, assuming you match with "all"); the
cache now has a negative entry.

2. The configured authentication method runs, and via a side
connection it creates the role "fred".

3. InitializeSessionUserId() looks up "fred", and finds the stale
negative entry.

I suppose the call to AcceptInvalidationMessages() could go at the end
of ClientAuthentication(). That'd be closer to the code that creates
the negative entry and immediately after the code that might modify
the catalogs. Or in PeformAuthentication(), its caller. Or in
IntializeSessionUserId() immediately before we try to look up the
name, but that's also called by background workers that don't need it.

--
Thomas Munro
http://www.enterprisedb.com