public inbox for [email protected]  
help / color / mirror / Atom feed
From: Maxim Orlov <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Maksim.Melnikov <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Jelte Fennema-Nio <[email protected]>
Subject: Re: Spinlock can be released twice in procsignal.c
Date: Wed, 26 Feb 2025 09:57:45 +0300
Message-ID: <CACG=ezbScO5O2CTZYy-Rqs-VHQ_f3aaRWL6VQZpyjC4uMVVDoA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>

On Wed, 26 Feb 2025 at 07:56, Michael Paquier <[email protected]> wrote:

>
> This fix is not correct.  No system function calls (well basically
> most of them) or even more no PostgreSQL-specific calls should happen
> while holding a spinlock.  elog() is a good example of what not to do.
> One example: imagine a palloc failure while holding this spinlock in
> this elog().


Indeed. PFA the correct one.


-- 
Best regards,
Maxim Orlov.


Attachments:

  [application/octet-stream] v2-0001-Avoid-double-spinlock-release.patch (1.6K, ../CACG=ezbScO5O2CTZYy-Rqs-VHQ_f3aaRWL6VQZpyjC4uMVVDoA@mail.gmail.com/3-v2-0001-Avoid-double-spinlock-release.patch)
  download | inline diff:
From ce41749e7df8339b5c1c90a42336f45d440028bb Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Wed, 26 Feb 2025 09:42:43 +0300
Subject: [PATCH v2] Avoid double spinlock release

---
 src/backend/storage/ipc/procsignal.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 7401b6e625e..6e9f9c0f2e5 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -167,6 +167,7 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
 {
 	ProcSignalSlot *slot;
 	uint64		barrier_generation;
+	bool		logit;
 
 	if (MyProcNumber < 0)
 		elog(ERROR, "MyProcNumber not set");
@@ -176,12 +177,7 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
 
 	/* sanity check */
 	SpinLockAcquire(&slot->pss_mutex);
-	if (pg_atomic_read_u32(&slot->pss_pid) != 0)
-	{
-		SpinLockRelease(&slot->pss_mutex);
-		elog(LOG, "process %d taking over ProcSignal slot %d, but it's not empty",
-			 MyProcPid, MyProcNumber);
-	}
+	logit = pg_atomic_read_u32(&slot->pss_pid) != 0;
 
 	/* Clear out any leftover signal reasons */
 	MemSet(slot->pss_signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t));
@@ -208,6 +204,12 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
 
 	SpinLockRelease(&slot->pss_mutex);
 
+	if (logit)
+	{
+		elog(LOG, "process %d taking over ProcSignal slot %d, but it's not empty",
+			 MyProcPid, MyProcNumber);
+	}
+
 	/* Remember slot location for CheckProcSignal */
 	MyProcSignalSlot = slot;
 
-- 
2.48.1



view thread (7+ messages)  latest in thread

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: Spinlock can be released twice in procsignal.c
  In-Reply-To: <CACG=ezbScO5O2CTZYy-Rqs-VHQ_f3aaRWL6VQZpyjC4uMVVDoA@mail.gmail.com>

* 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