On 2019-Oct-13, Justin Pryzby wrote:
> On Sun, Oct 13, 2019 at 03:10:21PM -0300, Alvaro Herrera wrote:
> > On 2019-Oct-13, Justin Pryzby wrote:
> >
> > > Looks like it's a race condition and dereferencing *holder=NULL. The
> > > first
> > > crash was probably the same bug, due to report query running during
> > > "reindex
> > > CONCURRENTLY", and probably finished at nearly the same time as another
> > > locker.
> >
> > Ooh, right, makes sense. There's another spot with the same mistake ...
> > this patch should fix it.
>
> I would maybe chop off the 2nd sentence, since conditionalizing indicates that
> we do actually care.
>
> + * If requested, publish who we're going to wait for.
> This is not
> + * 100% accurate if they're already gone, but we
> don't care.
True. And we can copy the resulting comment to the other spot.
(FWIW I expect the crash is possible not just in reindex but also in
CREATE INDEX CONCURRENTLY.)
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 70f9b6729a..589b8816a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -384,12 +384,14 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress)
if (VirtualTransactionIdIsValid(old_snapshots[i]))
{
+ /* If requested, publish who we're going to wait for. */
if (progress)
{
PGPROC *holder = BackendIdGetProc(old_snapshots[i].backendId);
- pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID,
- holder->pid);
+ if (holder)
+ pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID,
+ holder->pid);
}
VirtualXactLock(old_snapshots[i], true);
}
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 4682438114..1dd0e5e957 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -900,16 +900,14 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress)
while (VirtualTransactionIdIsValid(*lockholders))
{
- /*
- * If requested, publish who we're going to wait for. This is not
- * 100% accurate if they're already gone, but we don't care.
- */
+ /* If requested, publish who we're going to wait for. */
if (progress)
{
PGPROC *holder = BackendIdGetProc(lockholders->backendId);
- pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID,
- holder->pid);
+ if (holder)
+ pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID,
+ holder->pid);
}
VirtualXactLock(*lockholders, true);
lockholders++;
