public inbox for [email protected]  

help / color / mirror / Atom feed

From: Heikki Linnakangas <[email protected]>
To: Soumyadeep Chakraborty <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Sandeep Thakkar <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected]
Cc: CM Team <[email protected]>
Subject: Re: "ERROR: latch already owned" on gharial
Date: Thu, 8 Feb 2024 14:57:47 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAE-ML+_CL3TfhLo6MjCSufinyugqSJWr8qEoWL8oAc-oT+P67g@mail.gmail.com>
References: <CANFyU97otyhwE_Ut+59thA-J6rh-XxJvdfS3W9R50hSymsSxQA@mail.gmail.com>
	<[email protected]>
	<CAE-ML+_CL3TfhLo6MjCSufinyugqSJWr8qEoWL8oAc-oT+P67g@mail.gmail.com>

On 08/02/2024 04:08, Soumyadeep Chakraborty wrote:
> A possible ordering of events:
> 
> (1) DisownLatch() is called by pid Y during ProcKill() and the write for
> latch->owner_pid = 0 is NOT yet flushed to shmem.
> 
> (2) The PGPROC object for pid Y is returned to the free list.
> 
> (3) Pid X sees the same PGPROC object on the free list and grabs it.
> 
> (4) Pid X does sanity check inside OwnLatch during InitProcess and
> still sees the
> old value of latch->owner_pid = Y (and not = 0), and trips the ERROR.
> 
> The above sequence of operations should apply to PG HEAD as well.
> 
> Suggestion:
> 
> Should we do a pg_memory_barrier() at the end of DisownLatch(), like in
> ResetLatch(), like the one introduced in [3]? This would ensure that the write
> latch->owner_pid = 0; is flushed to shmem. The attached patch does this.

Hmm, there is a pair of SpinLockAcquire() and SpinLockRelease() in 
ProcKill(), before step 3 can happen. Comment in spin.h about 
SpinLockAcquire/Release:

>  *	Load and store operations in calling code are guaranteed not to be
>  *	reordered with respect to these operations, because they include a
>  *	compiler barrier.  (Before PostgreSQL 9.5, callers needed to use a
>  *	volatile qualifier to access data protected by spinlocks.)

That talks about a compiler barrier, though, not a memory barrier. But 
looking at the implementations in s_lock.h, I believe they do act as 
memory barrier, too.

So you might indeed have that problem on 9.4, but AFAICS not on later 
versions.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

view thread (2+ messages)

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: "ERROR: latch already owned" on gharial
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox