Re: [COMMITTERS] pgsql: Fix possible crash reading pg_stat_activity.

On Thu, Jan 5, 2017 at 12:37 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <rhaas(at)postgresql(dot)org> writes:
>> With the old code, a backend that read pg_stat_activity without ever
>> having executed a parallel query might see a backend in the midst of
>> executing one waiting on a DSA LWLock, resulting in a crash. The
>> solution is for backends to register the tranche at startup time, not
>> the first time a parallel query is executed.
>
> While I have no objection to the patch as committed, I have to wonder
> if this isn't papering over the underlying problem rather than solving it.
> It seems like this direction means that there's no such thing as dynamic
> registration of LWLock tranches and we should just give up on that concept
> entirely. If we do want to preserve the concept, don't we need to fix the
> pg_stat_activity code so it doesn't fail on tranches that aren't known
> locally?

It actually has such a safeguard already (see GetLWLockIdentifier).
Not that you mention it, I think I mis-stated the problem in the
commit message: the problem is not if the tranche is unregistered, but
rather if it is registered but the pointer references an address that
is no longer valid. Registering the tranche with a fixed string
rather than a pointer into a DSM segment that can go away fixes that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company