On Wed, Dec 28, 2016 at 10:40 AM, Thomas Munro <thomas.mu...@enterprisedb.com> wrote: > On Wed, Dec 28, 2016 at 10:01 AM, Andreas Seltenreich > <seltenre...@gmx.de> wrote: >> testing master as of fe591f8bf6 produced a crash reading >> pg_stat_activity (backtrace below). Digging around with with gdb >> revealed that pgstat_get_wait_event() returned an invalid pointer for a >> classId PG_WAIT_LWLOCK. >> >> I think the culprit is dsa.c passing a pointer to memory that goes away >> on dsa_free() as a name to LWLockRegisterTranche. > > I see, nice detective work! > > dsa.c's create_internal and attach_internal call LWLockRegisterTranche > with a pointer to a name in a DSM segment (copied from an argument to > dsa_create), which doesn't have the right extent in this rare race > case: the DSM segment goes away when the last backend detaches, but > after you've detached you might still see others waiting for a lock > that references that tranche ID in pg_stat_activity. This could be > fixed by copying the tranche name to somewhere with backend lifetime > extent in each backend, but then that would need cleanup. Maybe we > should replace it with another value when the DSA area is detached, > using a constant string. Something like > LWLockRegisterTranche(tranche_id, "detached_dsa"). That would be a > new use for the LWLockRegisterTranche function, for > re-registering/renaming and existing tranche ID. Thoughts?
Or we could introduce a new function UnregisterLWLockTranche(tranche_id) that is equivalent to RegisterLWLockTranche(tranche_id, NULL), and call that when detaching (ie in the hook that runs before we detach the control segment that holds the name). It looks like NULL would result in GetLWLockIdentifier returning "extension". I guess before DSA came along only extensions could create lock tranches not known to every backend, but perhaps now it should return "unknown"? It does seem a little odd that we are using a well known tranche ID defined in lwlock.h (as opposed to an ID obtained at runtime by LWLockNewTrancheId()) but the tranche name has to be registered at runtime in each backend and is unknown to other backends. That line of thinking suggests another potential solution: go and register the name in RegisterLWLockTranches, and stop registering it in dsa.c. For other potential uses of DSA including extensions that call LWLockNewTrancheId() we'd have to decide whether to make the name an optional argument, or require those users to register it themselves. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers