OSDN Git Service

Repair bugs discussed in pghackers thread of 15 May 1999: creation of a
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 30 Dec 1999 05:05:13 +0000 (05:05 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 30 Dec 1999 05:05:13 +0000 (05:05 +0000)
relcache entry no longer leaks a small amount of memory.  index_endscan
now releases all the memory acquired by index_beginscan, so callers of it
should NOT pfree the scan descriptor anymore.

src/backend/access/index/genam.c
src/backend/access/index/indexam.c
src/backend/catalog/indexing.c
src/backend/commands/trigger.c
src/backend/storage/large_object/inv_api.c
src/backend/utils/adt/regproc.c
src/backend/utils/cache/relcache.c
src/include/access/genam.h

index c197113..5eb0d78 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.21 1999/07/17 20:16:40 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.22 1999/12/30 05:04:50 tgl Exp $
  *
  * NOTES
  *       many of the old access method routines have been turned into
@@ -39,7 +39,7 @@
  * All other states cannot occur.
  *
  * Note:
- *It would be possible to cache the status of the previous and
+ *             It would be possible to cache the status of the previous and
  *             next item pointer using the flags.
  * ----------------------------------------------------------------
  */
  *             We don't know how the various AMs do locking, however, so we don't
  *             do anything about that here.
  *
- *             The intent is that an AM implementor will define a front-end routine
- *             that calls this one, to fill in the scan, and then does whatever kind
- *             of locking he wants.
+ *             The intent is that an AM implementor will define a beginscan routine
+ *             that calls RelationGetIndexScan, to fill in the scan, and then does
+ *             whatever kind of locking he wants.
+ *
+ *             At the end of a scan, the AM's endscan routine undoes the locking,
+ *             but does *not* call IndexScanEnd --- the higher-level index_endscan
+ *             routine does that.  (We can't do it in the AM because index_endscan
+ *             still needs to touch the IndexScanDesc after calling the AM.)
+ *
+ *             Because of this, the AM does not have a choice whether to call
+ *             RelationGetIndexScan or not; its beginscan routine must return an
+ *             object made by RelationGetIndexScan.  This is kinda ugly but not
+ *             worth cleaning up now.
  * ----------------------------------------------------------------
  */
 
  *                             relation -- index relation for scan.
  *                             scanFromEnd -- if true, begin scan at one of the index's
  *                                                        endpoints.
- *                             numberOfKeys -- count of scan keys (more than one won't
- *                                                             necessarily do anything useful, yet).
+ *                             numberOfKeys -- count of scan keys.
  *                             key -- the ScanKey for the starting position of the scan.
  *
  *             Returns:
  *                             An initialized IndexScanDesc.
- *
- *             Side Effects:
- *                             Bumps the ref count on the relation to keep it in the cache.
- *
  * ----------------
  */
 IndexScanDesc
@@ -118,6 +123,30 @@ RelationGetIndexScan(Relation relation,
        return scan;
 }
 
+/* ----------------
+ *     IndexScanEnd -- End an index scan.
+ *
+ *             This routine just releases the storage acquired by
+ *             RelationGetIndexScan().  Any AM-level resources are
+ *             assumed to already have been released by the AM's
+ *             endscan routine.
+ *
+ *     Returns:
+ *             None.
+ * ----------------
+ */
+void
+IndexScanEnd(IndexScanDesc scan)
+{
+       if (!IndexScanIsValid(scan))
+               elog(ERROR, "IndexScanEnd: invalid scan");
+
+       if (scan->keyData != NULL)
+               pfree(scan->keyData);
+
+       pfree(scan);
+}
+
 #ifdef NOT_USED
 /* ----------------
  *     IndexScanRestart -- Restart an index scan.
@@ -160,29 +189,6 @@ IndexScanRestart(IndexScanDesc scan,
 }
 
 /* ----------------
- *     IndexScanEnd -- End and index scan.
- *
- *             This routine is not used by any existing access method, but is
- *             suitable for use if you don't want to do sophisticated locking.
- *
- *     Returns:
- *             None.
- *
- *     Side Effects:
- *             None.
- * ----------------
- */
-void
-IndexScanEnd(IndexScanDesc scan)
-{
-       if (!IndexScanIsValid(scan))
-               elog(ERROR, "IndexScanEnd: invalid scan");
-
-       pfree(scan);
-}
-
-
-/* ----------------
  *     IndexScanMarkPosition -- Mark current position in a scan.
  *
  *             This routine isn't used by any existing access method, but is the
index 1a7ee4d..dc08d10 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/index/indexam.c,v 1.37 1999/11/07 23:07:54 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/index/indexam.c,v 1.38 1999/12/30 05:04:50 tgl Exp $
  *
  * INTERFACE ROUTINES
  *             index_open              - open an index relation by relationId
@@ -298,6 +298,9 @@ index_endscan(IndexScanDesc scan)
        UnlockRelation(scan->relation, AccessShareLock);
 
        RelationDecrementReferenceCount(scan->relation);
+
+       /* Release the scan data structure itself */
+       IndexScanEnd(scan);
 }
 
 /* ----------------
index 7918fbd..d7baec4 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/catalog/indexing.c,v 1.54 1999/12/16 22:19:39 wieck Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/catalog/indexing.c,v 1.55 1999/12/30 05:04:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -266,7 +266,7 @@ CatalogIndexFetchTuple(Relation heapRelation,
        }
 
        index_endscan(sd);
-       pfree(sd);
+
        return result;
 }
 
index 2615ea4..40159db 100644 (file)
@@ -513,7 +513,6 @@ RelationBuildTriggers(Relation relation)
                         NAMEDATALEN, RelationGetRelationName(relation));
 
        index_endscan(sd);
-       pfree(sd);
        index_close(irel);
        heap_close(tgrel, AccessShareLock);
 
@@ -1562,7 +1561,6 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt)
                        elog(ERROR, "Constraint '%s' does not exist", (char *)lfirst(l));
 
                index_endscan(sd);
-
        }
        index_close(irel);
        heap_close(tgrel, AccessShareLock);
index f066d41..5a28629 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/storage/large_object/inv_api.c,v 1.63 1999/12/16 22:19:51 wieck Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/storage/large_object/inv_api.c,v 1.64 1999/12/30 05:05:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -258,7 +258,6 @@ inv_close(LargeObjectDesc *obj_desc)
        if (obj_desc->iscan != (IndexScanDesc) NULL)
        {
                index_endscan(obj_desc->iscan);
-               pfree(obj_desc->iscan);
                obj_desc->iscan = NULL;
        }
 
@@ -583,7 +582,6 @@ inv_cleanindex(LargeObjectDesc *obj_desc)
                return;
 
        index_endscan(obj_desc->iscan);
-       pfree(obj_desc->iscan);
        obj_desc->iscan = (IndexScanDesc) NULL;
 
        ItemPointerSetInvalid(&(obj_desc->htid));
@@ -1255,7 +1253,6 @@ _inv_getsize(Relation hreln, TupleDesc hdesc, Relation ireln)
                if (res == (RetrieveIndexResult) NULL)
                {
                        index_endscan(iscan);
-                       pfree(iscan);
                        return 0;
                }
 
@@ -1271,7 +1268,6 @@ _inv_getsize(Relation hreln, TupleDesc hdesc, Relation ireln)
 
        /* don't need the index scan anymore */
        index_endscan(iscan);
-       pfree(iscan);
 
        /* get olastbyte attribute */
        d = heap_getattr(&tuple, 1, hdesc, &isNull);
index e346639..dffedd5 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/adt/regproc.c,v 1.46 1999/12/16 22:19:52 wieck Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/adt/regproc.c,v 1.47 1999/12/30 05:05:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -102,7 +102,6 @@ regprocin(char *pro_name_or_oid)
                        }
 
                        index_endscan(sd);
-                       pfree(sd);
                        index_close(idesc);
                        heap_close(hdesc, AccessShareLock);
 
index ffdc3cf..7eec36d 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.83 1999/12/28 13:40:49 wieck Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.84 1999/12/30 05:05:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -378,31 +378,27 @@ static Relation
 AllocateRelationDesc(Relation relation, u_int natts,
                                         Form_pg_class relp)
 {
-       Size            len;
        Form_pg_class relationForm;
 
        /* ----------------
         *      allocate space for the relation tuple form
         * ----------------
         */
-       relationForm = (Form_pg_class)
-               palloc((Size) (sizeof(FormData_pg_class)));
+       relationForm = (Form_pg_class) palloc(sizeof(FormData_pg_class));
 
-       memmove((char *) relationForm, (char *) relp, CLASS_TUPLE_SIZE);
+       memcpy((char *) relationForm, (char *) relp, CLASS_TUPLE_SIZE);
 
        /* ----------------
         *      allocate space for new relation descriptor, if needed
         */
-       len = sizeof(RelationData);
-
        if (relation == NULL)
-               relation = (Relation) palloc(len);
+               relation = (Relation) palloc(sizeof(RelationData));
 
        /* ----------------
         *      clear new reldesc
         * ----------------
         */
-       MemSet((char *) relation, 0, len);
+       MemSet((char *) relation, 0, sizeof(RelationData));
 
        /* make sure relation is marked as having no open file yet */
        relation->rd_fd = -1;
@@ -745,14 +741,10 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
        u_int           natts;
        Oid                     relid;
        Oid                     relam;
+       HeapTuple       pg_class_tuple;
        Form_pg_class relp;
-
        MemoryContext oldcxt;
 
-       HeapTuple       pg_class_tuple;
-
-       oldcxt = MemoryContextSwitchTo((MemoryContext) CacheCxt);
-
        /* ----------------
         *      find the tuple in pg_class corresponding to the given relation id
         * ----------------
@@ -764,11 +756,7 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
         * ----------------
         */
        if (!HeapTupleIsValid(pg_class_tuple))
-       {
-               MemoryContextSwitchTo(oldcxt);
-
                return NULL;
-       }
 
        /* ----------------
         *      get information from the pg_class_tuple
@@ -781,8 +769,10 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
        /* ----------------
         *      allocate storage for the relation descriptor,
         *      initialize relation->rd_rel and get the access method id.
+        *      The storage is allocated in memory context CacheCxt.
         * ----------------
         */
+       oldcxt = MemoryContextSwitchTo((MemoryContext) CacheCxt);
        relation = AllocateRelationDesc(oldrelation, natts, relp);
        relam = relation->rd_rel->relam;
 
@@ -866,6 +856,8 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
         */
        RelationCacheInsert(relation);
 
+       MemoryContextSwitchTo(oldcxt);
+
        /* -------------------
         *      free the memory allocated for pg_class_tuple
         *      and for lock data pointed to by pg_class_tuple
@@ -873,8 +865,6 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
         */
        heap_freetuple(pg_class_tuple);
 
-       MemoryContextSwitchTo(oldcxt);
-
        return relation;
 }
 
@@ -1764,7 +1754,6 @@ AttrDefaultFetch(Relation relation)
                         ndef - found, RelationGetRelationName(relation));
 
        index_endscan(sd);
-       pfree(sd);
        index_close(irel);
        heap_close(adrel, AccessShareLock);
 }
@@ -1837,7 +1826,6 @@ RelCheckFetch(Relation relation)
                         ncheck - found, RelationGetRelationName(relation));
 
        index_endscan(sd);
-       pfree(sd);
        index_close(irel);
        heap_close(rcrel, AccessShareLock);
 }
index fb98694..76200f2 100644 (file)
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: genam.h,v 1.20 1999/07/16 17:07:25 momjian Exp $
+ * $Id: genam.h,v 1.21 1999/12/30 05:05:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -47,5 +47,6 @@ extern Datum GetIndexValue(HeapTuple tuple, TupleDesc hTupDesc,
 /* in genam.c */
 extern IndexScanDesc RelationGetIndexScan(Relation relation, bool scanFromEnd,
                                         uint16 numberOfKeys, ScanKey key);
+extern void IndexScanEnd(IndexScanDesc scan);
 
 #endif  /* GENAM_H */