Horton

Re: BUG #17098: Assert failed on composing an error message when adding a type to an extension being dropped

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

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Date: 2021-07-11T15:28:58Z
Lists: pgsql-bugs, pgsql-hackers 
PG Bug reporting form <noreply@postgresql.org> writes:
> When trying to add to an extension a type that is already exists in the
> extension while the extension is being dropped I get a failed assertion with
> the following stack:

I think that the root issue here is that ExecAlterExtensionContentsStmt
fails to acquire any sort of lock on the extension.  Considering that
it *does* lock the object to be added/dropped, that's a rather glaring
oversight.  Fortunately it seems easily fixable ... though I wonder
how many other similar oversights we have.

However, that root issue is converted from a relatively minor bug into
a server crash because snprintf.c treats a NULL pointer passed to %s
as a crash-worthy error.  I have advocated for that behavior in the
past, but I'm starting to wonder if it wouldn't be wiser to change
over to the glibc-ish behavior of printing "(null)" or the like.
It seems like we've long since found all the interesting bugs that
the assert-or-crash behavior could reveal, and now we're down to
weird corner cases where its main effect is to weaken our robustness.

Thoughts?

			regards, tom lane

Commits

  1. Make printf("%s", NULL) print "(null)" instead of crashing.

  2. Lock the extension during ALTER EXTENSION ADD/DROP.

  3. Add an assertion that we don't pass NULL to snprintf("%s").

  4. Fix broken logic for reporting PL/Python function names in errcontext.