Message-ID: <790b901f-c0f6-4e97-b59a-97d8f921d57e@iki.fi>
Date: Mon, 29 Jul 2024 21:00:35 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Detect double-release of spinlock
To: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
References: <E1sYSF2-001lEB-D1@gemulon.postgresql.org>
 <2600757.1722267116@sss.pgh.pa.us>
 <20240729161846.kgiafbe2dtgabxgb@awork3.anarazel.de>
 <2802449.1722270793@sss.pgh.pa.us>
 <20240729164026.yurg37tej34o76uk@awork3.anarazel.de>
 <20240729165154.56zqyg34x2ywkpsh@awork3.anarazel.de>
 <e8d4ef94-b8aa-4cda-8f31-081f78194275@iki.fi>
 <2809049.1722273922@sss.pgh.pa.us>
 <20240729174853.uo5qjqkxlacjg6vv@awork3.anarazel.de>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <20240729174853.uo5qjqkxlacjg6vv@awork3.anarazel.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/790b901f-c0f6-4e97-b59a-97d8f921d57e%40iki.fi>
Precedence: bulk

On 29/07/2024 20:48, Andres Freund wrote:
> On 2024-07-29 13:25:22 -0400, Tom Lane wrote:
>> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>>> Yeah I'm not worried about that at all. Also, the assert is made when
>>> you have already released the spinlock; you are already out of the
>>> critical section.
>>
>> Not in the patch Andres posted.
> 
> Which seems fairly fundamental - once outside of the critical section, we
> can't actually assert that the lock isn't acquired, somebody else *validly*
> might have acquired it by then.

You could do:

bool was_free = S_LOCK_FREE(lock);

S_UNLOCK(lock);
Assert(!was_free);

Depending on the underlying implementation, you could also use 
compare-and-exchange. That makes the assertion-enabled instructions a 
little different than without assertions though.

> However, I still don't think it's a problem to assert that the lock is held in
> in the unlock "routine". As mentioned before, the spinlock implementation
> itself has never followed the "just straight line code" rule that users of
> spinlocks are supposed to follow.

Agreed.

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