From: Andres Freund <andres@anarazel.de>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18031: Segmentation fault after deadlock within VACUUM's parallel worker
Date: Mon, 24 Jul 2023 20:39:46 -0700
Message-ID: <20230725033946.mstudugbav7yghui@awork3.anarazel.de> (raw)
In-Reply-To: <CAD21AoDs7vzK7NErse7xTruqY-FXmM+3K00SdBtMcQhiRNkoeQ@mail.gmail.com>
References: <18031-ebe2d08cb405f6cc@postgresql.org>
<CAD21AoDs7vzK7NErse7xTruqY-FXmM+3K00SdBtMcQhiRNkoeQ@mail.gmail.com>
Hi,
Thanks for finding / debugging this issue!
On 2023-07-21 17:01:11 +0900, Masahiko Sawada wrote:
> I've reproduced the issue in my environment with the provided script.
> The crashed process is not a parallel vacuum worker actually but a
> parallel worker for rebuilding the index. The scenario seems that when
> detecting a deadlock, the process removes itself from the wait queue
> by RemoveFromWaitQueue() (called by CheckDeadLock()), and then
> RemoveFromWaitQueue() is called again by LockErrorCleanup() while
> aborting the transaction. With commit 5764f611e, in
> RemoveFromWaitQueue() we remove the process from the wait queue using
> dclist_delete_from():
>
> /* Remove proc from lock's wait queue */
> dclist_delete_from(&waitLock->waitProcs, &proc->links);
> :
> /* Clean up the proc's own state, and pass it the ok/fail signal */
> proc->waitLock = NULL;
> proc->waitProcLock = NULL;
> proc->waitStatus = PROC_WAIT_STATUS_ERROR;
>
> However, since dclist_delete_from() doesn't clear proc->links, in
> LockErrorCleanup(), dlist_node_is_detached() still returns false:
>
> if (!dlist_node_is_detached(&MyProc->links))
> {
> /* We could not have been granted the lock yet */
> RemoveFromWaitQueue(MyProc, lockAwaited->hashcode);
> }
Indeed :(
> leading to calling RemoveFromWaitQueue() again. I think we should use
> dclist_delete_from_thoroughly() instead. With the attached patch, the
> issue doesn't happen in my environment.
Yep. Do you want to push that fix, or should I?
> Another thing I noticed is that the Assert(waitLock) in
> RemoveFromWaitQueue() is useless actually, since we access *waitLock
> before that:
>
> I think we should fix it as well. This fix is also included in the
> attached patch.
Don't really have an opinion on that. It's been this way for longer, afaict.
Greetings,
Andres Freund
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: andres@anarazel.de, sawada.mshk@gmail.com, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18031: Segmentation fault after deadlock within VACUUM's parallel worker
In-Reply-To: <20230725033946.mstudugbav7yghui@awork3.anarazel.de>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox