Re: elog(DEBUG2 in SpinLocked section. - Mailing list pgsql-hackers

From Tom Lane
Subject Re: elog(DEBUG2 in SpinLocked section.
Date
Msg-id 972559.1591162071@sss.pgh.pa.us
Whole thread Raw
In response to Re: elog(DEBUG2 in SpinLocked section.  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: elog(DEBUG2 in SpinLocked section.
Re: elog(DEBUG2 in SpinLocked section.
List 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



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: OpenSSL 3.0.0 compatibility
Next
From: Michael Paquier
Date:
Subject: Re: elog(DEBUG2 in SpinLocked section.