Skip to content

Commit 80d76be

Browse files
tglsfdctglsfdc
committed
Avoid failure if autovacuum tries to access a just-dropped temp namespace.
Such an access became possible when commit 246a6c8 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 a052f6c. Backpatch as far as v11, as 246a6c8 was. Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com
1 parent 32bb453 commit 80d76be

File tree

3 files changed

+25
-14
lines changed

3 files changed

+25
-14
lines changed

‎src/backend/catalog/namespace.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3217,16 +3217,16 @@ isOtherTempNamespace(Oid namespaceId)
32173217
}
32183218

32193219
/*
3220-
* isTempNamespaceInUse - is the given namespace owned and actively used
3220+
* checkTempNamespaceStatus - is the given namespace owned and actively used
32213221
* by a backend?
32223222
*
32233223
* Note: this can be used while scanning relations in pg_class to detect
32243224
* orphaned temporary tables or namespaces with a backend connected to a
32253225
* given database. The result may be out of date quickly, so the caller
32263226
* must be careful how to handle this information.
32273227
*/
3228-
bool
3229-
isTempNamespaceInUse(Oid namespaceId)
3228+
TempNamespaceStatus
3229+
checkTempNamespaceStatus(Oid namespaceId)
32303230
{
32313231
PGPROC *proc;
32323232
int backendId;
@@ -3235,25 +3235,25 @@ isTempNamespaceInUse(Oid namespaceId)
32353235

32363236
backendId = GetTempNamespaceBackendId(namespaceId);
32373237

3238-
/* No such temporary namespace? */
3238+
/* No such namespace, or its name shows it's not temp? */
32393239
if (backendId == InvalidBackendId)
3240-
return false;
3240+
return TEMP_NAMESPACE_NOT_TEMP;
32413241

32423242
/* Is the backend alive? */
32433243
proc = BackendIdGetProc(backendId);
32443244
if (proc == NULL)
3245-
return false;
3245+
return TEMP_NAMESPACE_IDLE;
32463246

32473247
/* Is the backend connected to the same database we are looking at? */
32483248
if (proc->databaseId != MyDatabaseId)
3249-
return false;
3249+
return TEMP_NAMESPACE_IDLE;
32503250

32513251
/* Does the backend own the temporary namespace? */
32523252
if (proc->tempNamespaceId != namespaceId)
3253-
return false;
3253+
return TEMP_NAMESPACE_IDLE;
32543254

3255-
/* all good to go */
3256-
return true;
3255+
/* Yup, so namespace is busy */
3256+
return TEMP_NAMESPACE_IN_USE;
32573257
}
32583258

32593259
/*

‎src/backend/postmaster/autovacuum.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,9 +2071,10 @@ do_autovacuum(void)
20712071
{
20722072
/*
20732073
* We just ignore it if the owning backend is still active and
2074-
* using the temporary schema.
2074+
* using the temporary schema. Also, for safety, ignore it if the
2075+
* namespace doesn't exist or isn't a temp namespace after all.
20752076
*/
2076-
if (!isTempNamespaceInUse(classForm->relnamespace))
2077+
if (checkTempNamespaceStatus(classForm->relnamespace) == TEMP_NAMESPACE_IDLE)
20772078
{
20782079
/*
20792080
* The table seems to be orphaned -- although it might be that
@@ -2243,7 +2244,7 @@ do_autovacuum(void)
22432244
continue;
22442245
}
22452246

2246-
if (isTempNamespaceInUse(classForm->relnamespace))
2247+
if (checkTempNamespaceStatus(classForm->relnamespace) != TEMP_NAMESPACE_IDLE)
22472248
{
22482249
UnlockRelationOid(relid, AccessExclusiveLock);
22492250
continue;

‎src/include/catalog/namespace.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ typedef struct _FuncCandidateList
3737
Oid args[FLEXIBLE_ARRAY_MEMBER]; /* arg types */
3838
} *FuncCandidateList;
3939

40+
/*
41+
* Result of checkTempNamespaceStatus
42+
*/
43+
typedef enum TempNamespaceStatus
44+
{
45+
TEMP_NAMESPACE_NOT_TEMP, /* nonexistent, or non-temp namespace */
46+
TEMP_NAMESPACE_IDLE, /* exists, belongs to no active session */
47+
TEMP_NAMESPACE_IN_USE /* belongs to some active session */
48+
} TempNamespaceStatus;
49+
4050
/*
4151
* Structure for xxxOverrideSearchPath functions
4252
*/
@@ -138,7 +148,7 @@ extern bool isTempToastNamespace(Oid namespaceId);
138148
extern bool isTempOrTempToastNamespace(Oid namespaceId);
139149
extern bool isAnyTempNamespace(Oid namespaceId);
140150
extern bool isOtherTempNamespace(Oid namespaceId);
141-
extern bool isTempNamespaceInUse(Oid namespaceId);
151+
extern TempNamespaceStatus checkTempNamespaceStatus(Oid namespaceId);
142152
extern int GetTempNamespaceBackendId(Oid namespaceId);
143153
extern Oid GetTempToastNamespace(void);
144154
extern void GetTempNamespaceState(Oid *tempNamespaceId,

0 commit comments

Comments
 (0)