Horton

Re: elog(DEBUG2 in SpinLocked section.

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, masao.fujii@oss.nttdata.com, amit.kapila16@gmail.com, pasim@vmware.com, pgsql-hackers@lists.postgresql.org
Date: 2020-06-03T05:27:51Z
Lists: pgsql-hackers 
I wrote:
> Ugh, that is just horrid.  I experimented with the attached patch
> but it did not find any other problems.

It occurred to me to add NotHoldingSpinLock() into palloc and
friends, and look what I found in copy_replication_slot:

            SpinLockAcquire(&s->mutex);
            src_islogical = SlotIsLogical(s);
            src_restart_lsn = s->data.restart_lsn;
            temporary = s->data.persistency == RS_TEMPORARY;
            plugin = logical_slot ? pstrdup(NameStr(s->data.plugin)) : NULL;
            SpinLockRelease(&s->mutex);

That is not gonna do, of course.  And there is another pstrdup
inside another spinlock section a bit further down in the same
function.  Also, pg_get_replication_slots has a couple of
namecpy() calls inside a spinlock, which is maybe less dangerous
than palloc() but it's still willful disregard of the project coding
rule about "only straight-line code inside a spinlock".

I'm inclined to think that memcpy'ing the ReplicationSlot struct
into a local variable might be the best way, replacing all the
piecemeal copying these stanzas are doing right now.  memcpy() of
a fixed amount of data isn't quite straight-line code perhaps,
but it has a well-defined runtime and zero chance of throwing an
error, which are the two properties we should be most urgently
concerned about.

			regards, tom lane

Commits

  1. Fix instance of elog() called while holding a spinlock

  2. Don't call palloc() while holding a spinlock, either.

  3. Don't call elog() while holding spinlock.