aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2020-02-28 20:28:34 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2020-02-28 20:28:34 -0500
commitd7684c38a5e9db15c205371528ea3686235229f4 (patch)
treef96f0ff3299af7d6ceaf346a618ba8ec176185ec
parente198334034ef683558f857d513ff7f73f7c8c7bd (diff)
downloadpostgresql-d7684c38a5e9db15c205371528ea3686235229f4.tar.gz
postgresql-d7684c38a5e9db15c205371528ea3686235229f4.zip
Avoid failure if autovacuum tries to access a just-dropped temp namespace.
Such an access became possible when commit 246a6c8f7 added more aggressive cleanup of orphaned temp relations by autovacuum. Since autovacuum's snapshot might be slightly stale, it could attempt to access an already-dropped temp namespace, resulting in an assertion failure or null-pointer dereference. (In practice, since we don't drop temp namespaces automatically but merely recycle them, this situation could only arise if a superuser does a manual drop of a temp namespace. Still, that should be allowed.) The core of the bug, IMO, is that isTempNamespaceInUse and its callers failed to think hard about whether to treat "temp namespace isn't there" differently from "temp namespace isn't in use". In hopes of forestalling future mistakes of the same ilk, replace that function with a new one checkTempNamespaceStatus, which makes the same tests but returns a three-way enum rather than just a bool. isTempNamespaceInUse is gone entirely in HEAD; but just in case some external code is relying on it, keep it in the back branches, as a bug-compatible wrapper around the new function. Per report originally from Prabhat Kumar Sahu, investigated by Mahendra Singh and Michael Paquier; the final form of the patch is my fault. This replaces the failed fix attempt in a052f6cbb. Backpatch as far as v11, as 246a6c8f7 was. Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com
-rw-r--r--src/backend/catalog/namespace.c30
-rw-r--r--src/backend/postmaster/autovacuum.c7
-rw-r--r--src/include/catalog/namespace.h11
3 files changed, 35 insertions, 13 deletions
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index b8f0e1da6e9..48a70585796 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3217,7 +3217,7 @@ isOtherTempNamespace(Oid namespaceId)
}
/*
- * isTempNamespaceInUse - is the given namespace owned and actively used
+ * checkTempNamespaceStatus - is the given namespace owned and actively used
* by a backend?
*
* Note: this can be used while scanning relations in pg_class to detect
@@ -3225,8 +3225,8 @@ isOtherTempNamespace(Oid namespaceId)
* given database. The result may be out of date quickly, so the caller
* must be careful how to handle this information.
*/
-bool
-isTempNamespaceInUse(Oid namespaceId)
+TempNamespaceStatus
+checkTempNamespaceStatus(Oid namespaceId)
{
PGPROC *proc;
int backendId;
@@ -3235,25 +3235,35 @@ isTempNamespaceInUse(Oid namespaceId)
backendId = GetTempNamespaceBackendId(namespaceId);
- /* No such temporary namespace? */
+ /* No such namespace, or its name shows it's not temp? */
if (backendId == InvalidBackendId)
- return false;
+ return TEMP_NAMESPACE_NOT_TEMP;
/* Is the backend alive? */
proc = BackendIdGetProc(backendId);
if (proc == NULL)
- return false;
+ return TEMP_NAMESPACE_IDLE;
/* Is the backend connected to the same database we are looking at? */
if (proc->databaseId != MyDatabaseId)
- return false;
+ return TEMP_NAMESPACE_IDLE;
/* Does the backend own the temporary namespace? */
if (proc->tempNamespaceId != namespaceId)
- return false;
+ return TEMP_NAMESPACE_IDLE;
- /* all good to go */
- return true;
+ /* Yup, so namespace is busy */
+ return TEMP_NAMESPACE_IN_USE;
+}
+
+/*
+ * isTempNamespaceInUse - oversimplified, deprecated version of
+ * checkTempNamespaceStatus
+ */
+bool
+isTempNamespaceInUse(Oid namespaceId)
+{
+ return checkTempNamespaceStatus(namespaceId) == TEMP_NAMESPACE_IN_USE;
}
/*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 0b1e74f00bc..3377e64fd6d 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2076,9 +2076,10 @@ do_autovacuum(void)
{
/*
* We just ignore it if the owning backend is still active and
- * using the temporary schema.
+ * using the temporary schema. Also, for safety, ignore it if the
+ * namespace doesn't exist or isn't a temp namespace after all.
*/
- if (!isTempNamespaceInUse(classForm->relnamespace))
+ if (checkTempNamespaceStatus(classForm->relnamespace) == TEMP_NAMESPACE_IDLE)
{
/*
* The table seems to be orphaned -- although it might be that
@@ -2248,7 +2249,7 @@ do_autovacuum(void)
continue;
}
- if (isTempNamespaceInUse(classForm->relnamespace))
+ if (checkTempNamespaceStatus(classForm->relnamespace) != TEMP_NAMESPACE_IDLE)
{
UnlockRelationOid(relid, AccessExclusiveLock);
continue;
diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index 187d76de18a..692e27768e2 100644
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -38,6 +38,16 @@ typedef struct _FuncCandidateList
} *FuncCandidateList;
/*
+ * Result of checkTempNamespaceStatus
+ */
+typedef enum TempNamespaceStatus
+{
+ TEMP_NAMESPACE_NOT_TEMP, /* nonexistent, or non-temp namespace */
+ TEMP_NAMESPACE_IDLE, /* exists, belongs to no active session */
+ TEMP_NAMESPACE_IN_USE /* belongs to some active session */
+} TempNamespaceStatus;
+
+/*
* Structure for xxxOverrideSearchPath functions
*/
typedef struct OverrideSearchPath
@@ -138,6 +148,7 @@ extern bool isTempToastNamespace(Oid namespaceId);
extern bool isTempOrTempToastNamespace(Oid namespaceId);
extern bool isAnyTempNamespace(Oid namespaceId);
extern bool isOtherTempNamespace(Oid namespaceId);
+extern TempNamespaceStatus checkTempNamespaceStatus(Oid namespaceId);
extern bool isTempNamespaceInUse(Oid namespaceId);
extern int GetTempNamespaceBackendId(Oid namespaceId);
extern Oid GetTempToastNamespace(void);