Back-patch fix for race condition in heap_update (make sure we hold

the buffer lock while checking page free space).

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 49ec63658f2c7cd243ddb0a092334147510732a8..48e6c9f2724b3b494570aea0192151a6419b68d3 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.113 2001/03/25 23:23:58 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.113.2.1 2001/05/17 00:48:45 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1578,6 +1578,8 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
                                newbuf;
        bool            need_toast,
                                already_marked;
+       Size            newtupsize,
+                               pagefree;
        int                     result;
 
        /* increment access statistics */
@@ -1685,8 +1687,10 @@ l2:
                                  HeapTupleHasExtended(newtup) ||
                                  (MAXALIGN(newtup->t_len) > TOAST_TUPLE_THRESHOLD));
 
-       if (need_toast ||
-               (unsigned) MAXALIGN(newtup->t_len) > PageGetFreeSpace((Page) dp))
+       newtupsize = MAXALIGN(newtup->t_len);
+       pagefree = PageGetFreeSpace((Page) dp);
+
+       if (need_toast || newtupsize > pagefree)
        {
                _locked_tuple_.node = relation->rd_node;
                _locked_tuple_.tid = oldtup.t_self;
@@ -1704,17 +1708,59 @@ l2:
 
                /* Let the toaster do its thing */
                if (need_toast)
+               {
                        heap_tuple_toast_attrs(relation, newtup, &oldtup);
+                       newtupsize = MAXALIGN(newtup->t_len);
+               }
 
-               /* Now, do we need a new page for the tuple, or not? */
-               if ((unsigned) MAXALIGN(newtup->t_len) <= PageGetFreeSpace((Page) dp))
-                       newbuf = buffer;
-               else
+               /*
+                * Now, do we need a new page for the tuple, or not?  This is a bit
+                * tricky since someone else could have added tuples to the page
+                * while we weren't looking.  We have to recheck the available space
+                * after reacquiring the buffer lock.  But don't bother to do that
+                * if the former amount of free space is still not enough; it's
+                * unlikely there's more free now than before.
+                *
+                * What's more, if we need to get a new page, we will need to acquire
+                * buffer locks on both old and new pages.  To avoid deadlock against
+                * some other backend trying to get the same two locks in the other
+                * order, we must be consistent about the order we get the locks in.
+                * We use the rule "lock the higher-numbered page of the relation
+                * first".  To implement this, we must do RelationGetBufferForTuple
+                * while not holding the lock on the old page.  In 7.1, that routine
+                * always returns the last page of the relation, so the rule holds.
+                * (7.2 will use a different approach, but the same ordering rule.)
+                */
+               if (newtupsize > pagefree)
+               {
+                       /* Assume there's no chance to put newtup on same page. */
                        newbuf = RelationGetBufferForTuple(relation, newtup->t_len);
-
-               /* Re-acquire the lock on the old tuple's page. */
-               /* this seems to be deadlock free... */
-               LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+                       /* Now reacquire lock on old tuple's page. */
+                       LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+               }
+               else
+               {
+                       /* Re-acquire the lock on the old tuple's page. */
+                       LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+                       /* Re-check using the up-to-date free space */
+                       pagefree = PageGetFreeSpace((Page) dp);
+                       if (newtupsize > pagefree)
+                       {
+                               /*
+                                * Rats, it doesn't fit anymore.  We must now unlock and
+                                * relock to avoid deadlock.  Fortunately, this path should
+                                * seldom be taken.
+                                */
+                               LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+                               newbuf = RelationGetBufferForTuple(relation, newtup->t_len);
+                               LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+                       }
+                       else
+                       {
+                               /* OK, it fits here, so we're done. */
+                               newbuf = buffer;
+                       }
+               }
        }
        else
        {