diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c index a63e38e02ed..b429cf77834 100644 --- a/src/backend/libpq/be-fsstubs.c +++ b/src/backend/libpq/be-fsstubs.c @@ -8,15 +8,15 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/libpq/be-fsstubs.c,v 1.79.2.1 2005/11/22 18:23:09 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/libpq/be-fsstubs.c,v 1.79.2.2 2006/04/26 00:35:27 tgl Exp $ * * NOTES * This should be moved to a more appropriate place. It is here * for lack of a better place. * - * These functions operate in a private MemoryContext, which means - * that large object descriptors hang around until we destroy the context - * at transaction end. It'd be possible to prolong the lifetime + * These functions store LargeObjectDesc structs in a private MemoryContext, + * which means that large object descriptors hang around until we destroy + * the context at transaction end. It'd be possible to prolong the lifetime * of the context so that LO FDs are good across transactions (for example, * we could release the context only if we see that no FDs remain open). * But we'd need additional state in order to do the right thing at the @@ -29,8 +29,9 @@ * * As of PostgreSQL 8.0, much of the angst expressed above is no longer * relevant, and in fact it'd be pretty easy to allow LO FDs to stay - * open across transactions. However backwards compatibility suggests - * that we should stick to the status quo. + * open across transactions. (Snapshot relevancy would still be an issue.) + * However backwards compatibility suggests that we should stick to the + * status quo. * *------------------------------------------------------------------------- */ @@ -56,9 +57,9 @@ * LO "FD"s are indexes into the cookies array. * * A non-null entry is a pointer to a LargeObjectDesc allocated in the - * LO private memory context. The cookies array itself is also dynamically - * allocated in that context. Its current allocated size is cookies_len - * entries, of which any unused entries will be NULL. + * LO private memory context "fscxt". The cookies array itself is also + * dynamically allocated in that context. Its current allocated size is + * cookies_len entries, of which any unused entries will be NULL. */ static LargeObjectDesc **cookies = NULL; static int cookies_size = 0; @@ -91,7 +92,6 @@ lo_open(PG_FUNCTION_ARGS) int32 mode = PG_GETARG_INT32(1); LargeObjectDesc *lobjDesc; int fd; - MemoryContext currentContext; #if FSDB elog(DEBUG4, "lo_open(%u,%d)", lobjId, mode); @@ -99,13 +99,10 @@ lo_open(PG_FUNCTION_ARGS) CreateFSContext(); - currentContext = MemoryContextSwitchTo(fscxt); - - lobjDesc = inv_open(lobjId, mode); + lobjDesc = inv_open(lobjId, mode, fscxt); if (lobjDesc == NULL) { /* lookup failed */ - MemoryContextSwitchTo(currentContext); #if FSDB elog(DEBUG4, "could not open large object %u", lobjId); #endif @@ -114,8 +111,6 @@ lo_open(PG_FUNCTION_ARGS) fd = newLOfd(lobjDesc); - MemoryContextSwitchTo(currentContext); - PG_RETURN_INT32(fd); } @@ -123,7 +118,6 @@ Datum lo_close(PG_FUNCTION_ARGS) { int32 fd = PG_GETARG_INT32(0); - MemoryContext currentContext; if (fd < 0 || fd >= cookies_size || cookies[fd] == NULL) { @@ -136,15 +130,10 @@ lo_close(PG_FUNCTION_ARGS) elog(DEBUG4, "lo_close(%d)", fd); #endif - Assert(fscxt != NULL); - currentContext = MemoryContextSwitchTo(fscxt); - inv_close(cookies[fd]); deleteLOfd(fd); - MemoryContextSwitchTo(currentContext); - PG_RETURN_INT32(0); } @@ -160,7 +149,6 @@ lo_close(PG_FUNCTION_ARGS) int lo_read(int fd, char *buf, int len) { - MemoryContext currentContext; int status; if (fd < 0 || fd >= cookies_size || cookies[fd] == NULL) @@ -171,20 +159,14 @@ lo_read(int fd, char *buf, int len) return -1; } - Assert(fscxt != NULL); - currentContext = MemoryContextSwitchTo(fscxt); - status = inv_read(cookies[fd], buf, len); - MemoryContextSwitchTo(currentContext); - return status; } int lo_write(int fd, char *buf, int len) { - MemoryContext currentContext; int status; if (fd < 0 || fd >= cookies_size || cookies[fd] == NULL) @@ -201,13 +183,8 @@ lo_write(int fd, char *buf, int len) errmsg("large object descriptor %d was not opened for writing", fd))); - Assert(fscxt != NULL); - currentContext = MemoryContextSwitchTo(fscxt); - status = inv_write(cookies[fd], buf, len); - MemoryContextSwitchTo(currentContext); - return status; } @@ -218,7 +195,6 @@ lo_lseek(PG_FUNCTION_ARGS) int32 fd = PG_GETARG_INT32(0); int32 offset = PG_GETARG_INT32(1); int32 whence = PG_GETARG_INT32(2); - MemoryContext currentContext; int status; if (fd < 0 || fd >= cookies_size || cookies[fd] == NULL) @@ -229,13 +205,8 @@ lo_lseek(PG_FUNCTION_ARGS) PG_RETURN_INT32(-1); } - Assert(fscxt != NULL); - currentContext = MemoryContextSwitchTo(fscxt); - status = inv_seek(cookies[fd], offset, whence); - MemoryContextSwitchTo(currentContext); - PG_RETURN_INT32(status); } @@ -243,17 +214,15 @@ Datum lo_creat(PG_FUNCTION_ARGS) { Oid lobjId; - MemoryContext currentContext; - /* do we actually need fscxt for this? */ + /* + * We don't actually need to store into fscxt, but create it anyway to + * ensure that AtEOXact_LargeObject knows there is state to clean up + */ CreateFSContext(); - currentContext = MemoryContextSwitchTo(fscxt); - lobjId = inv_create(InvalidOid); - MemoryContextSwitchTo(currentContext); - PG_RETURN_OID(lobjId); } @@ -261,17 +230,15 @@ Datum lo_create(PG_FUNCTION_ARGS) { Oid lobjId = PG_GETARG_OID(0); - MemoryContext currentContext; - /* do we actually need fscxt for this? */ + /* + * We don't actually need to store into fscxt, but create it anyway to + * ensure that AtEOXact_LargeObject knows there is state to clean up + */ CreateFSContext(); - currentContext = MemoryContextSwitchTo(fscxt); - lobjId = inv_create(lobjId); - MemoryContextSwitchTo(currentContext); - PG_RETURN_OID(lobjId); } @@ -288,10 +255,6 @@ lo_tell(PG_FUNCTION_ARGS) PG_RETURN_INT32(-1); } - /* - * We assume we do not need to switch contexts for inv_tell. That is true - * for now, but is probably more than this module ought to assume... - */ PG_RETURN_INT32(inv_tell(cookies[fd])); } @@ -305,10 +268,8 @@ lo_unlink(PG_FUNCTION_ARGS) */ if (fscxt != NULL) { - MemoryContext currentContext; int i; - currentContext = MemoryContextSwitchTo(fscxt); for (i = 0; i < cookies_size; i++) { if (cookies[i] != NULL && cookies[i]->id == lobjId) @@ -317,13 +278,11 @@ lo_unlink(PG_FUNCTION_ARGS) deleteLOfd(i); } } - MemoryContextSwitchTo(currentContext); } /* - * inv_drop does not need a context switch, indeed it doesn't touch any - * LO-specific data structures at all. (Again, that's probably more than - * this module ought to be assuming.) + * inv_drop does not create a need for end-of-transaction cleanup and + * hence we don't need to have created fscxt. */ PG_RETURN_INT32(inv_drop(lobjId)); } @@ -391,10 +350,6 @@ lo_import(PG_FUNCTION_ARGS) errhint("Anyone can use the client-side lo_import() provided by libpq."))); #endif - /* - * We don't actually need to switch into fscxt, but create it anyway to - * ensure that AtEOXact_LargeObject knows there is state to clean up - */ CreateFSContext(); /* @@ -420,7 +375,7 @@ lo_import(PG_FUNCTION_ARGS) /* * read in from the filesystem and write to the inversion object */ - lobj = inv_open(lobjOid, INV_WRITE); + lobj = inv_open(lobjOid, INV_WRITE, fscxt); while ((nbytes = FileRead(fd, buf, BUFSIZE)) > 0) { @@ -465,16 +420,12 @@ lo_export(PG_FUNCTION_ARGS) errhint("Anyone can use the client-side lo_export() provided by libpq."))); #endif - /* - * We don't actually need to switch into fscxt, but create it anyway to - * ensure that AtEOXact_LargeObject knows there is state to clean up - */ CreateFSContext(); /* * open the inversion object (no need to test for failure) */ - lobj = inv_open(lobjId, INV_READ); + lobj = inv_open(lobjId, INV_READ, fscxt); /* * open the file to be written to @@ -524,13 +475,10 @@ void AtEOXact_LargeObject(bool isCommit) { int i; - MemoryContext currentContext; if (fscxt == NULL) return; /* no LO operations in this xact */ - currentContext = MemoryContextSwitchTo(fscxt); - /* * Close LO fds and clear cookies array so that LO fds are no longer good. * On abort we skip the close step. @@ -549,8 +497,6 @@ AtEOXact_LargeObject(bool isCommit) cookies = NULL; cookies_size = 0; - MemoryContextSwitchTo(currentContext); - /* Release the LO memory context to prevent permanent memory leaks. */ MemoryContextDelete(fscxt); fscxt = NULL; @@ -623,8 +569,7 @@ newLOfd(LargeObjectDesc *lobjCookie) i = 0; newsize = 64; cookies = (LargeObjectDesc **) - palloc(newsize * sizeof(LargeObjectDesc *)); - MemSet(cookies, 0, newsize * sizeof(LargeObjectDesc *)); + MemoryContextAllocZero(fscxt, newsize * sizeof(LargeObjectDesc *)); cookies_size = newsize; } else diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c index 74409f3cd0a..8c8e5c8b56e 100644 --- a/src/backend/storage/large_object/inv_api.c +++ b/src/backend/storage/large_object/inv_api.c @@ -4,12 +4,20 @@ * routines for manipulating inversion fs large objects. This file * contains the user-level large object application interface routines. * + * + * Note: many of these routines leak memory in CurrentMemoryContext, as indeed + * does most of the backend code. We expect that CurrentMemoryContext will + * be a short-lived context. Data that must persist across function calls + * is kept either in CacheMemoryContext (the Relation structs) or in the + * memory context given to inv_open (for LargeObjectDesc structs). + * + * * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.113 2005/10/15 02:49:26 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.113.2.1 2006/04/26 00:35:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -205,14 +213,17 @@ inv_create(Oid lobjId) * inv_open -- access an existing large object. * * Returns: - * large object descriptor, appropriately filled in. + * Large object descriptor, appropriately filled in. The descriptor + * and subsidiary data are allocated in the specified memory context, + * which must be suitably long-lived for the caller's purposes. */ LargeObjectDesc * -inv_open(Oid lobjId, int flags) +inv_open(Oid lobjId, int flags, MemoryContext mcxt) { LargeObjectDesc *retval; - retval = (LargeObjectDesc *) palloc(sizeof(LargeObjectDesc)); + retval = (LargeObjectDesc *) MemoryContextAlloc(mcxt, + sizeof(LargeObjectDesc)); retval->id = lobjId; retval->subid = GetCurrentSubTransactionId(); @@ -225,9 +236,12 @@ inv_open(Oid lobjId, int flags) } else if (flags & INV_READ) { - /* be sure to copy snap into fscxt */ + /* be sure to copy snap into mcxt */ + MemoryContext oldContext = MemoryContextSwitchTo(mcxt); + retval->snapshot = CopySnapshot(ActiveSnapshot); retval->flags = IFS_RDLOCK; + MemoryContextSwitchTo(oldContext); } else elog(ERROR, "invalid flags: %d", flags); @@ -242,7 +256,8 @@ inv_open(Oid lobjId, int flags) } /* - * Closes an existing large object descriptor. + * Closes a large object descriptor previously made by inv_open(), and + * releases the long-term memory used by it. */ void inv_close(LargeObjectDesc *obj_desc) diff --git a/src/include/storage/large_object.h b/src/include/storage/large_object.h index c9795d2f7a3..e64558130ee 100644 --- a/src/include/storage/large_object.h +++ b/src/include/storage/large_object.h @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/large_object.h,v 1.32 2005/06/13 02:26:52 tgl Exp $ + * $PostgreSQL: pgsql/src/include/storage/large_object.h,v 1.32.2.1 2006/04/26 00:35:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -71,7 +71,7 @@ typedef struct LargeObjectDesc /* inversion stuff in inv_api.c */ extern void close_lo_relation(bool isCommit); extern Oid inv_create(Oid lobjId); -extern LargeObjectDesc *inv_open(Oid lobjId, int flags); +extern LargeObjectDesc *inv_open(Oid lobjId, int flags, MemoryContext mcxt); extern void inv_close(LargeObjectDesc *obj_desc); extern int inv_drop(Oid lobjId); extern int inv_seek(LargeObjectDesc *obj_desc, int offset, int whence);