Fix race condition with toast table access from a stale syscache entry.

If a tuple in a syscache contains an out-of-line toasted field, and we
try to fetch that field shortly after some other transaction has committed
an update or deletion of the tuple, there is a race condition: vacuum
could come along and remove the toast tuples before we can fetch them.
This leads to transient failures like "missing chunk number 0 for toast
value NNNNN in pg_toast_2619", as seen in recent reports from Andrew
Hammond and Tim Uckun.

The design idea of syscache is that access to stale syscache entries
should be prevented by relation-level locks, but that fails for at least
two cases where toasted fields are possible: ANALYZE updates pg_statistic
rows without locking out sessions that might want to plan queries on the
same table, and CREATE OR REPLACE FUNCTION updates pg_proc rows without
any meaningful lock at all.

The least risky fix seems to be an idea that Heikki suggested when we
were dealing with a related problem back in August: forcibly detoast any
out-of-line fields before putting a tuple into syscache in the first place.
This avoids the problem because at the time we fetch the parent tuple from
the catalog, we should be holding an MVCC snapshot that will prevent
removal of the toast tuples, even if the parent tuple is outdated
immediately after we fetch it.  (Note: I'm not convinced that this
statement holds true at every instant where we could be fetching a syscache
entry at all, but it does appear to hold true at the times where we could
fetch an entry that could have a toasted field.  We will need to be a bit
wary of adding toast tables to low-level catalogs that don't have them
already.)  An additional benefit is that subsequent uses of the syscache
entry should be faster, since they won't have to detoast the field.

Back-patch to all supported versions.  The problem is significantly harder
to reproduce in pre-9.0 releases, because of their willingness to flush
every entry in a syscache whenever the underlying catalog is vacuumed
(cf CatalogCacheFlushRelation); but there is still a window for trouble.

diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 0ce04a5727561fc9e045dd5b84ab317a6a637cc5..b13bc8d270d26409c0f93080b15e346dbe9f6464 100644 (file)
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -928,6 +928,87 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 }
 
 
+/* ----------
+ * toast_flatten_tuple -
+ *
+ *     "Flatten" a tuple to contain no out-of-line toasted fields.
+ *     (This does not eliminate compressed or short-header datums.)
+ * ----------
+ */
+HeapTuple
+toast_flatten_tuple(HeapTuple tup, TupleDesc tupleDesc)
+{
+       HeapTuple       new_tuple;
+       Form_pg_attribute *att = tupleDesc->attrs;
+       int                     numAttrs = tupleDesc->natts;
+       int                     i;
+       Datum           toast_values[MaxTupleAttributeNumber];
+       bool            toast_isnull[MaxTupleAttributeNumber];
+       bool            toast_free[MaxTupleAttributeNumber];
+
+       /*
+        * Break down the tuple into fields.
+        */
+       Assert(numAttrs <= MaxTupleAttributeNumber);
+       heap_deform_tuple(tup, tupleDesc, toast_values, toast_isnull);
+
+       memset(toast_free, 0, numAttrs * sizeof(bool));
+
+       for (i = 0; i < numAttrs; i++)
+       {
+               /*
+                * Look at non-null varlena attributes
+                */
+               if (!toast_isnull[i] && att[i]->attlen == -1)
+               {
+                       struct varlena *new_value;
+
+                       new_value = (struct varlena *) DatumGetPointer(toast_values[i]);
+                       if (VARATT_IS_EXTERNAL(new_value))
+                       {
+                               new_value = toast_fetch_datum(new_value);
+                               toast_values[i] = PointerGetDatum(new_value);
+                               toast_free[i] = true;
+                       }
+               }
+       }
+
+       /*
+        * Form the reconfigured tuple.
+        */
+       new_tuple = heap_form_tuple(tupleDesc, toast_values, toast_isnull);
+
+       /*
+        * Be sure to copy the tuple's OID and identity fields.  We also make a
+        * point of copying visibility info, just in case anybody looks at those
+        * fields in a syscache entry.
+        */
+       if (tupleDesc->tdhasoid)
+               HeapTupleSetOid(new_tuple, HeapTupleGetOid(tup));
+
+       new_tuple->t_self = tup->t_self;
+       new_tuple->t_tableOid = tup->t_tableOid;
+
+       new_tuple->t_data->t_choice = tup->t_data->t_choice;
+       new_tuple->t_data->t_ctid = tup->t_data->t_ctid;
+       new_tuple->t_data->t_infomask &= ~HEAP_XACT_MASK;
+       new_tuple->t_data->t_infomask |=
+               tup->t_data->t_infomask & HEAP_XACT_MASK;
+       new_tuple->t_data->t_infomask2 &= ~HEAP2_XACT_MASK;
+       new_tuple->t_data->t_infomask2 |=
+               tup->t_data->t_infomask2 & HEAP2_XACT_MASK;
+
+       /*
+        * Free allocated temp values
+        */
+       for (i = 0; i < numAttrs; i++)
+               if (toast_free[i])
+                       pfree(DatumGetPointer(toast_values[i]));
+
+       return new_tuple;
+}
+
+
 /* ----------
  * toast_flatten_tuple_attribute -
  *

diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index f43e4181e781190222a6e82fcec9482c9366fc64..5242cdd115025b54b44ae9566a7e4f4a90432c73 100644 (file)
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -19,6 +19,7 @@
 #include "access/heapam.h"
 #include "access/relscan.h"
 #include "access/sysattr.h"
+#include "access/tuptoaster.h"
 #include "access/valid.h"
 #include "catalog/pg_operator.h"
 #include "catalog/pg_type.h"
@@ -1591,16 +1592,32 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp,
                                                uint32 hashValue, Index hashIndex, bool negative)
 {
        CatCTup    *ct;
+       HeapTuple       dtp;
        MemoryContext oldcxt;
 
+       /*
+        * If there are any out-of-line toasted fields in the tuple, expand them
+        * in-line.  This saves cycles during later use of the catcache entry,
+        * and also protects us against the possibility of the toast tuples being
+        * freed before we attempt to fetch them, in case of something using a
+        * slightly stale catcache entry.
+        */
+       if (HeapTupleHasExternal(ntp))
+               dtp = toast_flatten_tuple(ntp, cache->cc_tupdesc);
+       else
+               dtp = ntp;
+
        /*
         * Allocate CatCTup header in cache memory, and copy the tuple there too.
         */
        oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
        ct = (CatCTup *) palloc(sizeof(CatCTup));
-       heap_copytuple_with_tuple(ntp, &ct->tuple);
+       heap_copytuple_with_tuple(dtp, &ct->tuple);
        MemoryContextSwitchTo(oldcxt);
 
+       if (dtp != ntp)
+               heap_freetuple(dtp);
+
        /*
         * Finish initializing the CatCTup header, and add it to the cache's
         * linked list and counts.

diff --git a/src/include/access/tuptoaster.h b/src/include/access/tuptoaster.h
index 1ae44e0e695e72b0ad4f7971135992ec3a04b612..4e3e3ca4ba3895549e5c6128c8c48825ba41ec13 100644 (file)
--- a/src/include/access/tuptoaster.h
+++ b/src/include/access/tuptoaster.h
@@ -143,6 +143,15 @@ extern struct varlena *heap_tuple_untoast_attr_slice(struct varlena * attr,
                                                          int32 sliceoffset,
                                                          int32 slicelength);
 
+/* ----------
+ * toast_flatten_tuple -
+ *
+ *     "Flatten" a tuple to contain no out-of-line toasted fields.
+ *     (This does not eliminate compressed or short-header datums.)
+ * ----------
+ */
+extern HeapTuple toast_flatten_tuple(HeapTuple tup, TupleDesc tupleDesc);
+
 /* ----------
  * toast_flatten_tuple_attribute -
  *