References: <87plyhvama.fsf@163.com>
 <CA+Tgmob=RA_y4shbtodwRRhC9uwDSWnBHY=3EvnJ_txtAWoD9A@mail.gmail.com>
 <489987.1704381735@sss.pgh.pa.us>
 <CA+Tgmoa=q9KZ-2u8Cx35D_7HSa8YKzvfA5ffvd876_UZ2LEU3Q@mail.gmail.com>
 <498328.1704386013@sss.pgh.pa.us>
 <CA+TgmoaA2uqsozTj9UDNrXTKWiUz1kW9eQsKiCr9ZvPwaFGFDA@mail.gmail.com>
 <20240104225403.dgmbbfffmm3srpgq@awork3.anarazel.de>
 <871qaso7ub.fsf@163.com>
 <CA+TgmoabLxQCbnatgdcO_VZKQtZqnj4c0zg3ovavqBZBh+QYcA@mail.gmail.com>
User-agent: mu4e 1.10.7; emacs 29.1
From: Andy Fan <zhihuifan1213@163.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Thomas
 Munro <tmunro@postgresql.org>, pgsql-hackers@lists.postgresql.org
Subject: Re: the s_lock_stuck on perform_spin_delay
Date: Tue, 09 Jan 2024 10:01:59 +0800
In-reply-to: 
 <CA+TgmoabLxQCbnatgdcO_VZKQtZqnj4c0zg3ovavqBZBh+QYcA@mail.gmail.com>
Message-ID: <87sf37mdqq.fsf@163.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/87sf37mdqq.fsf%40163.com>
Precedence: bulk


Hi,
Robert Haas <robertmhaas@gmail.com> writes:

> On Sun, Jan 7, 2024 at 9:52=E2=80=AFPM Andy Fan <zhihuifan1213@163.com> w=
rote:
>> > I think we should add cassert-only infrastructure tracking whether we
>> > currently hold spinlocks, are in a signal handler and perhaps a few ot=
her
>> > states. That'd allow us to add assertions like:
>> ..
>> > - no lwlocks or ... while in signal handlers
>>
>> I *wish* lwlocks should *not* be held while in signal handlers since it
>> inspired me for a direction of a low-frequency internal bug where a
>> backend acuqire a LWLock when it has acuqired it before. However when I
>> read more document and code, I am thinking this should not be a
>> problem.
>
> It's not safe to acquire an LWLock in a signal handler unless we know
> that the code that the signal handler is interrupting can't already be
> doing that. Consider this code from LWLockAcquire:

Thanks for the explaination! I can follow the sistuation you descirbe
here, then I found I asked a bad question because I didn't clarify what
"signal handlers" I was refering to, sorry about that!

In your statement, I guess you are talking about the signal handler from
Linux. However I *assumed* such handlers are doing pretty similar stuff
like set a 'GlobalVarialbe=3Dtrue'. If my assumption was right, I think
that should not be take cared. For example:

spin_or_lwlock_acquire();
... (linux signal handler may be invovked here no matther what ... code is)
spin_or_lwlock_relase()

Since the linux signal hander are pretty simply, so it can come back to
'spin_or_lwlock_relase' anyway. (However my assumption may be wrong and
thanks for highlight this, and it is helpful for me to debug my internal
bug!)

The singler handler I was refering to is 'CHECK_FOR_INTERRUPTS', Based
on this, spin_lock and lwlock are acted pretty differently.=20

spin_lock_acuqire();
CHECK_FOR_INTERRUPT();
spin_lock_release();

Since CHECK_FOR_INTERRUPT usually goes to the ERROR system which makes it
is hard to go back to 'spin_lock_release()', then spin lock leaks! so
CHECK_FOR_INTERRUPT is the place I Assert *spin lock* should not be
handled in my patch. and I understood what Andres was talking about is
the same thing. (Of course I can add the "Assert no spin lock is held"
into every linux single handler as well).

Based on the above, I asked my question in my previous post, where I am
not sure if we should do the same('Assert no-lwlock should be held') for
*lwlock* in CHECK_FOR_INTERRUPT since lwlocks can be released no matter
where CHECK_FOR_INTERRUPT jump to.

--=20
Best Regards
Andy Fan