database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-04-27 22:56:53 +03:00
Code

Fix two violations of the ResourceOwnerEnlarge/Remember protocol.

Browse Source 
The point of having separate ResourceOwnerEnlargeFoo and
ResourceOwnerRememberFoo functions is so that resource allocation
can happen in between.  Doing it in some other order is just wrong.

OpenTemporaryFile() did open(), enlarge, remember, which would leak the
open file if the enlarge step ran out of memory.  Because fd.c has its own
layer of resource-remembering, the consequences look like they'd be limited
to an intratransaction FD leak, but it's still not good.

IncrBufferRefCount() did enlarge, remember, incr-refcount, which would blow
up if the incr-refcount step ever failed.  It was safe enough when written,
but since the introduction of PrivateRefCountHash, I think the assumption
that no error could happen there is pretty shaky.

The odds of real problems from either bug are probably small, but still,
back-patch to supported branches.

Thomas Munro and Tom Lane, per a comment from Andres Freund
This commit is contained in:
Tom Lane 2017-11-08 16:50:13 -05:00
parent 7a15fe5a22
commit 1786cdab11
2 changed files with 9 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/backend/storage/buffer/bufmgr.c
View File

@ -2629,11 +2629,11 @@ IncrBufferRefCount(Buffer buffer)
{ {
Assert(BufferIsPinned(buffer)); Assert(BufferIsPinned(buffer));
ResourceOwnerEnlargeBuffers(CurrentResourceOwner); ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
ResourceOwnerRememberBuffer(CurrentResourceOwner, buffer);
if (BufferIsLocal(buffer)) if (BufferIsLocal(buffer))
LocalRefCount[-buffer - 1]++; LocalRefCount[-buffer - 1]++;
else else
PrivateRefCount[buffer - 1]++; PrivateRefCount[buffer - 1]++;
ResourceOwnerRememberBuffer(CurrentResourceOwner, buffer);
} }
/* /*

10
src/backend/storage/file/fd.c
View File

@ -1193,6 +1193,13 @@ OpenTemporaryFile(bool interXact)
{ {
File file = 0; File file = 0;
/*
* Make sure the current resource owner has space for this File before we
* open it, if we'll be registering it below.
*/
if (!interXact)
ResourceOwnerEnlargeFiles(CurrentResourceOwner);
/* /*
* If some temp tablespace(s) have been given to us, try to use the next * If some temp tablespace(s) have been given to us, try to use the next
* one. If a given tablespace can't be found, we silently fall back to * one. If a given tablespace can't be found, we silently fall back to
@ -1229,9 +1236,8 @@ OpenTemporaryFile(bool interXact)
{ {
VfdCache[file].fdstate |= FD_XACT_TEMPORARY; VfdCache[file].fdstate |= FD_XACT_TEMPORARY;
ResourceOwnerEnlargeFiles(CurrentResourceOwner);
ResourceOwnerRememberFile(CurrentResourceOwner, file);
VfdCache[file].resowner = CurrentResourceOwner; VfdCache[file].resowner = CurrentResourceOwner;
ResourceOwnerRememberFile(CurrentResourceOwner, file);
/* ensure cleanup happens at eoxact */ /* ensure cleanup happens at eoxact */
have_xact_temporary_files = true; have_xact_temporary_files = true;