Index: src/backend/libpq/be-fsstubs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/be-fsstubs.c,v
retrieving revision 1.79.2.1
diff -c -r1.79.2.1 be-fsstubs.c
*** src/backend/libpq/be-fsstubs.c	22 Nov 2005 18:23:09 -0000	1.79.2.1
--- src/backend/libpq/be-fsstubs.c	26 Apr 2006 00:23:12 -0000
***************
*** 14,22 ****
   *	  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
   *	  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
--- 14,22 ----
   *	  This should be moved to a more appropriate place.  It is here
   *	  for lack of a better place.
   *
!  *	  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,36 ****
   *
   *	  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.
   *
   *-------------------------------------------------------------------------
   */
--- 29,37 ----
   *
   *	  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.  (Snapshot relevancy would still be an issue.)
!  *	  However backwards compatibility suggests that we should stick to the
!  *	  status quo.
   *
   *-------------------------------------------------------------------------
   */
***************
*** 56,64 ****
   * 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.
   */
  static LargeObjectDesc **cookies = NULL;
  static int	cookies_size = 0;
--- 57,65 ----
   * 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 "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,97 ****
  	int32		mode = PG_GETARG_INT32(1);
  	LargeObjectDesc *lobjDesc;
  	int			fd;
- 	MemoryContext currentContext;
  
  #if FSDB
  	elog(DEBUG4, "lo_open(%u,%d)", lobjId, mode);
--- 92,97 ----
***************
*** 99,111 ****
  
  	CreateFSContext();
  
! 	currentContext = MemoryContextSwitchTo(fscxt);
! 
! 	lobjDesc = inv_open(lobjId, mode);
  
  	if (lobjDesc == NULL)
  	{							/* lookup failed */
- 		MemoryContextSwitchTo(currentContext);
  #if FSDB
  		elog(DEBUG4, "could not open large object %u", lobjId);
  #endif
--- 99,108 ----
  
  	CreateFSContext();
  
! 	lobjDesc = inv_open(lobjId, mode, fscxt);
  
  	if (lobjDesc == NULL)
  	{							/* lookup failed */
  #if FSDB
  		elog(DEBUG4, "could not open large object %u", lobjId);
  #endif
***************
*** 114,121 ****
  
  	fd = newLOfd(lobjDesc);
  
- 	MemoryContextSwitchTo(currentContext);
- 
  	PG_RETURN_INT32(fd);
  }
  
--- 111,116 ----
***************
*** 123,129 ****
  lo_close(PG_FUNCTION_ARGS)
  {
  	int32		fd = PG_GETARG_INT32(0);
- 	MemoryContext currentContext;
  
  	if (fd < 0 || fd >= cookies_size || cookies[fd] == NULL)
  	{
--- 118,123 ----
***************
*** 136,150 ****
  	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);
  }
  
--- 130,139 ----
***************
*** 160,166 ****
  int
  lo_read(int fd, char *buf, int len)
  {
- 	MemoryContext currentContext;
  	int			status;
  
  	if (fd < 0 || fd >= cookies_size || cookies[fd] == NULL)
--- 149,154 ----
***************
*** 171,190 ****
  		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)
--- 159,172 ----
***************
*** 201,213 ****
  			  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;
  }
  
--- 183,190 ----
***************
*** 218,224 ****
  	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)
--- 195,200 ----
***************
*** 229,241 ****
  		PG_RETURN_INT32(-1);
  	}
  
- 	Assert(fscxt != NULL);
- 	currentContext = MemoryContextSwitchTo(fscxt);
- 
  	status = inv_seek(cookies[fd], offset, whence);
  
- 	MemoryContextSwitchTo(currentContext);
- 
  	PG_RETURN_INT32(status);
  }
  
--- 205,212 ----
***************
*** 243,259 ****
  lo_creat(PG_FUNCTION_ARGS)
  {
  	Oid			lobjId;
- 	MemoryContext currentContext;
  
! 	/* do we actually need fscxt for this? */
  	CreateFSContext();
  
- 	currentContext = MemoryContextSwitchTo(fscxt);
- 
  	lobjId = inv_create(InvalidOid);
  
- 	MemoryContextSwitchTo(currentContext);
- 
  	PG_RETURN_OID(lobjId);
  }
  
--- 214,228 ----
  lo_creat(PG_FUNCTION_ARGS)
  {
  	Oid			lobjId;
  
! 	/*
! 	 * 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();
  
  	lobjId = inv_create(InvalidOid);
  
  	PG_RETURN_OID(lobjId);
  }
  
***************
*** 261,277 ****
  lo_create(PG_FUNCTION_ARGS)
  {
  	Oid			lobjId = PG_GETARG_OID(0);
- 	MemoryContext currentContext;
  
! 	/* do we actually need fscxt for this? */
  	CreateFSContext();
  
- 	currentContext = MemoryContextSwitchTo(fscxt);
- 
  	lobjId = inv_create(lobjId);
  
- 	MemoryContextSwitchTo(currentContext);
- 
  	PG_RETURN_OID(lobjId);
  }
  
--- 230,244 ----
  lo_create(PG_FUNCTION_ARGS)
  {
  	Oid			lobjId = PG_GETARG_OID(0);
  
! 	/*
! 	 * 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();
  
  	lobjId = inv_create(lobjId);
  
  	PG_RETURN_OID(lobjId);
  }
  
***************
*** 288,297 ****
  		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]));
  }
  
--- 255,260 ----
***************
*** 305,314 ****
  	 */
  	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)
--- 268,275 ----
***************
*** 317,329 ****
  				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.)
  	 */
  	PG_RETURN_INT32(inv_drop(lobjId));
  }
--- 278,288 ----
  				deleteLOfd(i);
  			}
  		}
  	}
  
  	/*
! 	 * 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,400 ****
  				 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();
  
  	/*
--- 350,355 ----
***************
*** 420,426 ****
  	/*
  	 * read in from the filesystem and write to the inversion object
  	 */
! 	lobj = inv_open(lobjOid, INV_WRITE);
  
  	while ((nbytes = FileRead(fd, buf, BUFSIZE)) > 0)
  	{
--- 375,381 ----
  	/*
  	 * read in from the filesystem and write to the inversion object
  	 */
! 	lobj = inv_open(lobjOid, INV_WRITE, fscxt);
  
  	while ((nbytes = FileRead(fd, buf, BUFSIZE)) > 0)
  	{
***************
*** 465,480 ****
  				 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);
  
  	/*
  	 * open the file to be written to
--- 420,431 ----
  				 errhint("Anyone can use the client-side lo_export() provided by libpq.")));
  #endif
  
  	CreateFSContext();
  
  	/*
  	 * open the inversion object (no need to test for failure)
  	 */
! 	lobj = inv_open(lobjId, INV_READ, fscxt);
  
  	/*
  	 * open the file to be written to
***************
*** 524,536 ****
  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.
--- 475,484 ----
***************
*** 549,556 ****
  	cookies = NULL;
  	cookies_size = 0;
  
- 	MemoryContextSwitchTo(currentContext);
- 
  	/* Release the LO memory context to prevent permanent memory leaks. */
  	MemoryContextDelete(fscxt);
  	fscxt = NULL;
--- 497,502 ----
***************
*** 623,630 ****
  		i = 0;
  		newsize = 64;
  		cookies = (LargeObjectDesc **)
! 			palloc(newsize * sizeof(LargeObjectDesc *));
! 		MemSet(cookies, 0, newsize * sizeof(LargeObjectDesc *));
  		cookies_size = newsize;
  	}
  	else
--- 569,575 ----
  		i = 0;
  		newsize = 64;
  		cookies = (LargeObjectDesc **)
! 			MemoryContextAllocZero(fscxt, newsize * sizeof(LargeObjectDesc *));
  		cookies_size = newsize;
  	}
  	else
Index: src/backend/storage/large_object/inv_api.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/storage/large_object/inv_api.c,v
retrieving revision 1.113
diff -c -r1.113 inv_api.c
*** src/backend/storage/large_object/inv_api.c	15 Oct 2005 02:49:26 -0000	1.113
--- src/backend/storage/large_object/inv_api.c	26 Apr 2006 00:23:12 -0000
***************
*** 4,9 ****
--- 4,17 ----
   *	  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
   *
***************
*** 205,218 ****
   *	inv_open -- access an existing large object.
   *
   *		Returns:
!  *		  large object descriptor, appropriately filled in.
   */
  LargeObjectDesc *
! inv_open(Oid lobjId, int flags)
  {
  	LargeObjectDesc *retval;
  
! 	retval = (LargeObjectDesc *) palloc(sizeof(LargeObjectDesc));
  
  	retval->id = lobjId;
  	retval->subid = GetCurrentSubTransactionId();
--- 213,229 ----
   *	inv_open -- access an existing large object.
   *
   *		Returns:
!  *		  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, MemoryContext mcxt)
  {
  	LargeObjectDesc *retval;
  
! 	retval = (LargeObjectDesc *) MemoryContextAlloc(mcxt,
! 													sizeof(LargeObjectDesc));
  
  	retval->id = lobjId;
  	retval->subid = GetCurrentSubTransactionId();
***************
*** 225,233 ****
  	}
  	else if (flags & INV_READ)
  	{
! 		/* be sure to copy snap into fscxt */
  		retval->snapshot = CopySnapshot(ActiveSnapshot);
  		retval->flags = IFS_RDLOCK;
  	}
  	else
  		elog(ERROR, "invalid flags: %d", flags);
--- 236,247 ----
  	}
  	else if (flags & INV_READ)
  	{
! 		/* 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,248 ****
  }
  
  /*
!  * Closes an existing large object descriptor.
   */
  void
  inv_close(LargeObjectDesc *obj_desc)
--- 256,263 ----
  }
  
  /*
!  * 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)
Index: src/include/storage/large_object.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/storage/large_object.h,v
retrieving revision 1.32
diff -c -r1.32 large_object.h
*** src/include/storage/large_object.h	13 Jun 2005 02:26:52 -0000	1.32
--- src/include/storage/large_object.h	26 Apr 2006 00:23:12 -0000
***************
*** 71,77 ****
  /* 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 void inv_close(LargeObjectDesc *obj_desc);
  extern int	inv_drop(Oid lobjId);
  extern int	inv_seek(LargeObjectDesc *obj_desc, int offset, int whence);
--- 71,77 ----
  /* 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, 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);
