Theses buffer leaks are caused by indexes that are kept open between

calls. Outside a transaction, the backend detects them as buffer
leaks; it sends a NOTICE, and frees them. This sometimes cause a
segmentation fault (at least on Linux). These indexes are initialized
on the first lo_read/lo_write/lo_tell call, and (normally) closed
on a lo_close call.  Thus the buffer leaks appear when lo direct
access functions are used, and not with lo_import/lo_export functions
(libpq version calls lo_close before ending the command, and the
backend version uses another path).

The included patches (against recent snapshot, and against 6.3.2)
cause indexes to be closed on transaction end (that is on explicit
'END' statment, or on command termination outside trasaction blocks),
thus preventing the buffer leaks while increasing performance inside
transactions. Some (all?) 'classic' memory leaks are also removed.

I hope it will be ok.

--- Pascal ANDRE, graduated from Ecole Centrale Paris andre@via.ecp.fr

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index fb888640a9951061b300f2a6bf21b10b46f7f0cc..f32a7fb00b5ab37549976da92cc398b5c407e6ea 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.20 1998/06/15 19:28:02 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.21 1998/07/21 04:17:21 momjian Exp $
  *
  * NOTES
  *             Transaction aborts can now occur two ways:
@@ -137,6 +137,11 @@
  *-------------------------------------------------------------------------
  */
 
+/*
+ * Large object clean up added in CommitTransaction() to prevent buffer leaks.
+ * [PA, 7/17/98]
+ * [PA] is Pascal André <andre@via.ecp.fr>
+ */
 #include <postgres.h>
 
 #include <access/xact.h>
@@ -151,6 +156,9 @@
 #include <commands/async.h>
 #include <commands/sequence.h>
 
+/* included for _lo_commit [PA, 7/17/98] */
+#include <libpq/be-fsstubs.h>
+
 static void AbortTransaction(void);
 static void AtAbort_Cache(void);
 static void AtAbort_Locks(void);
@@ -889,6 +897,10 @@ CommitTransaction()
         *      do commit processing
         * ----------------
         */
+
+       /* handle commit for large objects [ PA, 7/17/98 ] */ 
+       _lo_commit();
+
        CloseSequences();
        DestroyTempRels();
        AtEOXact_portals();

diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 8ecf45fe1225d36430ab2087fde7744e3143d151..5267cf40abe2c95a6b3ed73dde805fadc8150cea 100644 (file)
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/libpq/be-fsstubs.c,v 1.21 1998/06/15 19:28:25 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/libpq/be-fsstubs.c,v 1.22 1998/07/21 04:17:23 momjian Exp $
  *
  * NOTES
  *       This should be moved to a more appropriate place.  It is here
@@ -41,6 +41,8 @@
 #include <storage/large_object.h>
 #include <libpq/be-fsstubs.h>
 
+/* [PA] is Pascal André <andre@via.ecp.fr> */
+
 /*#define FSDB 1*/
 #define MAX_LOBJ_FDS 256
 
@@ -52,7 +54,6 @@ static GlobalMemory fscxt = NULL;
 static int     newLOfd(LargeObjectDesc *lobjCookie);
 static void deleteLOfd(int fd);
 
-
 /*****************************************************************************
  *     File Interfaces for Large Objects
  *****************************************************************************/
@@ -367,6 +368,26 @@ lo_export(Oid lobjId, text *filename)
        return 1;
 }
 
+/*
+ * lo_commit -
+ *       prepares large objects for transaction commit [PA, 7/17/98]
+ */
+void 
+_lo_commit(void)
+{
+        int i;
+       MemoryContext currentContext;
+
+       currentContext = MemoryContextSwitchTo((MemoryContext) fscxt);
+
+        for (i = 0; i < MAX_LOBJ_FDS; i++) {
+                if (cookies[i] != NULL) inv_cleanindex(cookies[i]);
+        }
+
+       MemoryContextSwitchTo(currentContext);
+
+}
+
 
 /*****************************************************************************
  *     Support routines for this file

diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c
index c3d63ac36b2cd2baa7783e69bc64cf927ac7159c..e92c75f0a346408ddccd7a9f1a3c860641f69c5e 100644 (file)
--- a/src/backend/storage/large_object/inv_api.c
+++ b/src/backend/storage/large_object/inv_api.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/storage/large_object/inv_api.c,v 1.30 1998/06/15 19:29:16 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/storage/large_object/inv_api.c,v 1.31 1998/07/21 04:17:24 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -60,6 +60,19 @@
  *     for data.
  */
 
+/*
+ *      In order to prevent buffer leak on transaction commit, large object
+ *      scan index handling has been modified. Indexes are persistant inside
+ *      a transaction but may be closed between two calls to this API (when 
+ *      transaction is committed while object is opened, or when no 
+ *      transaction is active). Scan indexes are thus now reinitialized using
+ *      the object current offset. [PA]
+ *
+ *      Some cleanup has been also done for non freed memory.
+ *
+ *      For subsequent notes, [PA] is Pascal André <andre@via.ecp.fr>
+ */
+
 #define IFREESPC(p)            (PageGetFreeSpace(p) - sizeof(HeapTupleData) - sizeof(struct varlena) - sizeof(int32))
 #define IMAXBLK                        8092
 #define IMINBLK                        512
@@ -261,8 +274,11 @@ inv_close(LargeObjectDesc *obj_desc)
 {
        Assert(PointerIsValid(obj_desc));
 
-       if (obj_desc->iscan != (IndexScanDesc) NULL)
+       if (obj_desc->iscan != (IndexScanDesc) NULL) {
                index_endscan(obj_desc->iscan);
+               pfree(obj_desc->iscan);
+               obj_desc->iscan = NULL;
+       }
 
        heap_close(obj_desc->heap_r);
        index_close(obj_desc->index_r);
@@ -546,6 +562,28 @@ inv_write(LargeObjectDesc *obj_desc, char *buf, int nbytes)
        return (nwritten);
 }
 
+/*
+ * inv_cleanindex --
+ *       Clean opened indexes for large objects, and clears current result.
+ *       This is necessary on transaction commit in order to prevent buffer
+ *       leak. 
+ *       This function must be called for each opened large object.
+ *       [ PA, 7/17/98 ]
+ */
+void 
+inv_cleanindex(LargeObjectDesc *obj_desc)
+{
+        Assert(PointerIsValid(obj_desc));
+
+       if (obj_desc->iscan == (IndexScanDesc) NULL) return;
+
+       index_endscan(obj_desc->iscan);
+       pfree(obj_desc->iscan);
+       obj_desc->iscan = (IndexScanDesc) NULL;
+       
+       ItemPointerSetInvalid(&(obj_desc->htid));
+}
+
 /*
  *     inv_fetchtup -- Fetch an inversion file system block.
  *
@@ -591,8 +629,13 @@ inv_fetchtup(LargeObjectDesc *obj_desc, Buffer *bufP)
                {
                        ScanKeyData skey;
 
+                       /* 
+                        * As scan index may be prematurely closed (on commit),
+                        * we must use object current offset (was 0) to 
+                        * reinitialize the entry [ PA ].
+                        */
                        ScanKeyEntryInitialize(&skey, 0x0, 1, F_INT4GE,
-                                                                  Int32GetDatum(0));
+                                              Int32GetDatum(obj_desc->offset));
                        obj_desc->iscan =
                                index_beginscan(obj_desc->index_r,
                                                                (bool) 0, (uint16) 1,
@@ -622,7 +665,7 @@ inv_fetchtup(LargeObjectDesc *obj_desc, Buffer *bufP)
 
                /* remember this tid -- we may need it for later reads/writes */
                ItemPointerCopy(&(res->heap_iptr), &(obj_desc->htid));
-
+               pfree(res);
        }
        else
        {
@@ -1179,6 +1222,7 @@ _inv_getsize(Relation hreln, TupleDesc hdesc, Relation ireln)
                if (res == (RetrieveIndexResult) NULL)
                {
                        index_endscan(iscan);
+                       pfree(iscan);
                        return (0);
                }
 
@@ -1192,11 +1236,13 @@ _inv_getsize(Relation hreln, TupleDesc hdesc, Relation ireln)
                        ReleaseBuffer(buf);
 
                htup = heap_fetch(hreln, false, &(res->heap_iptr), &buf);
+               pfree(res);
 
        } while (!HeapTupleIsValid(htup));
 
        /* don't need the index scan anymore */
        index_endscan(iscan);
+       pfree(iscan);
 
        /* get olastbyte attribute */
        d = heap_getattr(htup, 1, hdesc, &isNull);

diff --git a/src/include/libpq/be-fsstubs.h b/src/include/libpq/be-fsstubs.h
index 507f2a936778f8f9314a4e25cdf988c27d366621..108869dac955e50652fed317f5bf5f988dc70f90 100644 (file)
--- a/src/include/libpq/be-fsstubs.h
+++ b/src/include/libpq/be-fsstubs.h
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: be-fsstubs.h,v 1.5 1997/09/08 21:52:33 momjian Exp $
+ * $Id: be-fsstubs.h,v 1.6 1998/07/21 04:17:26 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,4 +37,9 @@ extern int    lo_unlink(Oid lobjId);
 extern struct varlena *loread(int fd, int len);
 extern int     lowrite(int fd, struct varlena * wbuf);
 
+/*
+ * Added for buffer leak prevention [ Pascal André <andre@via.ecp.fr> ]
+ */
+extern void _lo_commit(void);
+
 #endif                                                 /* BE_FSSTUBS_H */

diff --git a/src/include/storage/large_object.h b/src/include/storage/large_object.h
index 276792e19e209d668790f7e0ea2df4e010849719..3d35dba907af27d513ba4e64c2b8c3b6a4777a23 100644 (file)
--- a/src/include/storage/large_object.h
+++ b/src/include/storage/large_object.h
@@ -7,7 +7,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: large_object.h,v 1.7 1997/09/08 21:54:29 momjian Exp $
+ * $Id: large_object.h,v 1.8 1998/07/21 04:17:30 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -54,4 +54,7 @@ extern int    inv_tell(LargeObjectDesc *obj_desc);
 extern int     inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes);
 extern int     inv_write(LargeObjectDesc *obj_desc, char *buf, int nbytes);
 
+/* added for buffer leak prevention [ PA ] */
+extern void inv_cleanindex(LargeObjectDesc *obj_desc);
+
 #endif                                                 /* LARGE_OBJECT_H */