Avoid crash in rare case of concurrent DROP
authorAlvaro Herrera <[email protected]>
Fri, 5 Nov 2021 15:29:35 +0000 (12:29 -0300)
committerAlvaro Herrera <[email protected]>
Fri, 5 Nov 2021 15:29:35 +0000 (12:29 -0300)
When a role being dropped contains is referenced by catalog objects that
are concurrently also being dropped, a crash can result while trying to
construct the string that describes the objects.  Suppress that by
ignoring objects whose descriptions are returned as NULL.

The majority of relevant codesites were already cautious about this
already; we had just missed a couple.

This is an old bug, so backpatch all the way back.

Reported-by: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/17126-21887f04508cb5c8@postgresql.org

src/backend/catalog/dependency.c
src/backend/catalog/pg_shdepend.c

index 743d91c1a11ea8a92b2d8c8e7d2fb962cf11157b..60977f310f88369fcb289a0c7d09b29f4b2068f6 100644 (file)
@@ -905,6 +905,10 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
 
        objDesc = getObjectDescription(obj);
 
+       /* An object being dropped concurrently doesn't need to be reported */
+       if (objDesc == NULL)
+           continue;
+
        /*
         * If, at any stage of the recursive search, we reached the object via
         * an AUTO, INTERNAL, or EXTENSION dependency, then it's okay to
@@ -928,23 +932,28 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
        {
            char       *otherDesc = getObjectDescription(&extra->dependee);
 
-           if (numReportedClient < MAX_REPORTED_DEPS)
+           if (otherDesc)
            {
+               if (numReportedClient < MAX_REPORTED_DEPS)
+               {
+                   /* separate entries with a newline */
+                   if (clientdetail.len != 0)
+                       appendStringInfoChar(&clientdetail, '\n');
+                   appendStringInfo(&clientdetail, _("%s depends on %s"),
+                                    objDesc, otherDesc);
+                   numReportedClient++;
+               }
+               else
+                   numNotReportedClient++;
                /* separate entries with a newline */
-               if (clientdetail.len != 0)
-                   appendStringInfoChar(&clientdetail, '\n');
-               appendStringInfo(&clientdetail, _("%s depends on %s"),
+               if (logdetail.len != 0)
+                   appendStringInfoChar(&logdetail, '\n');
+               appendStringInfo(&logdetail, _("%s depends on %s"),
                                 objDesc, otherDesc);
-               numReportedClient++;
+               pfree(otherDesc);
            }
            else
                numNotReportedClient++;
-           /* separate entries with a newline */
-           if (logdetail.len != 0)
-               appendStringInfoChar(&logdetail, '\n');
-           appendStringInfo(&logdetail, _("%s depends on %s"),
-                            objDesc, otherDesc);
-           pfree(otherDesc);
            ok = false;
        }
        else
index 02f5591afae47ec4e59f9d6e765cd0dd55069c9c..167b9558adb336088baea5c351eafd917fb8b2b4 100644 (file)
@@ -1073,6 +1073,12 @@ storeObjectDescription(StringInfo descs,
 {
    char       *objdesc = getObjectDescription(object);
 
+   /*
+    * An object being dropped concurrently doesn't need to be reported.
+    */
+   if (objdesc == NULL)
+       return;
+
    /* separate entries with a newline */
    if (descs->len != 0)
        appendStringInfoChar(descs, '\n');