Fix bogus lock release in RemovePolicyById and RemoveRoleFromObjectPolicy.
authorTom Lane
Mon, 4 Jan 2016 01:53:35 +0000 (20:53 -0500)
committerTom Lane
Mon, 4 Jan 2016 01:53:41 +0000 (20:53 -0500)
Can't release the AccessExclusiveLock on the target table until commit.
Otherwise there is a race condition whereby other backends might service
our cache invalidation signals before they can actually see the updated
catalog rows.

Just to add insult to injury, RemovePolicyById was closing the rel (with
incorrect lock drop) and then passing the now-dangling rel pointer to
CacheInvalidateRelcache.  Probably the reason this doesn't fall over on
CLOBBER_CACHE buildfarm members is that some outer level of the DROP logic
is still holding the rel open ... but it'd have bit us on the arse
eventually, no doubt.

src/backend/commands/policy.c

index 6b9c3065b4f9ab1d3661a34f7ae47316e23b30f4..5c73dfc8c4619534cb3a8ef19810029d78b3a3b9 100644 (file)
@@ -370,7 +370,10 @@ RemovePolicyById(Oid policy_id)
        elog(ERROR, "could not find tuple for policy %u", policy_id);
 
    /*
-    * Open and exclusive-lock the relation the policy belong to.
+    * Open and exclusive-lock the relation the policy belongs to.  (We need
+    * exclusive lock to lock out queries that might otherwise depend on the
+    * set of policies the rel has; furthermore we've got to hold the lock
+    * till commit.)
     */
    relid = ((Form_pg_policy) GETSTRUCT(tuple))->polrelid;
 
@@ -390,7 +393,6 @@ RemovePolicyById(Oid policy_id)
    simple_heap_delete(pg_policy_rel, &tuple->t_self);
 
    systable_endscan(sscan);
-   heap_close(rel, AccessExclusiveLock);
 
    /*
     * Note that, unlike some of the other flags in pg_class, relrowsecurity
@@ -400,9 +402,10 @@ RemovePolicyById(Oid policy_id)
     * policy is created and all records are filtered (except for queries from
     * the owner).
     */
-
    CacheInvalidateRelcache(rel);
 
+   heap_close(rel, NoLock);
+
    /* Clean up */
    heap_close(pg_policy_rel, RowExclusiveLock);
 }
@@ -657,7 +660,9 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
 
    /* Clean up. */
    systable_endscan(sscan);
-   relation_close(rel, AccessExclusiveLock);
+
+   relation_close(rel, NoLock);
+
    heap_close(pg_policy_rel, RowExclusiveLock);
 
    return(noperm || num_roles > 0);
@@ -734,7 +739,7 @@ CreatePolicy(CreatePolicyStmt *stmt)
                                        RangeVarCallbackForPolicy,
                                        (void *) stmt);
 
-   /* Open target_table to build quals. No lock is necessary. */
+   /* Open target_table to build quals. No additional lock is necessary. */
    target_table = relation_open(table_id, NoLock);
 
    /* Add for the regular security quals */