On Thu, Apr 21, 2016 at 2:04 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, Apr 20, 2016 at 10:56 PM, Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote: >> At Wed, 20 Apr 2016 15:14:16 +0200, Petr Jelinek <p...@2ndquadrant.com> >> wrote in <571780a8.4070...@2ndquadrant.com> >>> I noticed sporadic segfaults when selecting from pg_stat_activity on >>> current HEAD. >>> >>> The culprit is the 53be0b1add7064ca5db3cd884302dfc3268d884e commit >>> which added more wait info into the pg_stat_get_activity(). More >>> specifically, the following code is broken: >>> >>> + proc = BackendPidGetProc(beentry->st_procpid); >>> + wait_event_type = pgstat_get_wait_event_type(proc->wait_event_info); >>> >>> This needs to check if proc is NULL. When reading the code I noticed >>> that the new functions pg_stat_get_backend_wait_event_type() and >>> pg_stat_get_backend_wait_event() suffer from the same problem. >> >> Good catch. > > Agreed. > >>> Here is PoC patch which fixes the problem. I am wondering if we should >>> raise warning in the pg_stat_get_backend_wait_event_type() and >>> pg_stat_get_backend_wait_event() like the pg_signal_backend() does >>> when proc is NULL instead of just returning NULL which is what this >>> patch does though. >> >> It still makes the two relevant columns in pg_stat_activity >> inconsistent each other since it reads the procarray entry twice >> without a lock on procarray. >> >> The attached patch adds pgstat_get_wait_event_info to read >> wait_event_info in more appropriate way. Then change >> pg_stat_get_wait_event(_type) to take the wait_event_info. >> >> Does this work for you? > > This is a hideously way of fixing this problem. The whole point of > storing the wait event in a 4-byte integer is that we already assume > reads of 4 byte integers are atomic and thus no lock is needed. The > only thing we need to do is to prevent the value from being read > twice, and we already have precedent for how to prevent that in > freelist.c.
That was intended to say "a hideously expensive way". -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers