summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane2017-11-08 21:50:13 +0000
committerTom Lane2017-11-08 21:50:13 +0000
commit1786cdab113701f154ca804537fe1c21d23e4ce8 (patch)
tree55db98b7a88d42b0679d0d65a676aba1cbde3f5c
parent7a15fe5a2240244e2f80f25adfe6b9a34787a021 (diff)
Fix two violations of the ResourceOwnerEnlarge/Remember protocol.
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
-rw-r--r--src/backend/storage/buffer/bufmgr.c2
-rw-r--r--src/backend/storage/file/fd.c10
2 files changed, 9 insertions, 3 deletions
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 2803159bc58..d9adcf7fe6a 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -2629,11 +2629,11 @@ IncrBufferRefCount(Buffer buffer)
{
Assert(BufferIsPinned(buffer));
ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
- ResourceOwnerRememberBuffer(CurrentResourceOwner, buffer);
if (BufferIsLocal(buffer))
LocalRefCount[-buffer - 1]++;
else
PrivateRefCount[buffer - 1]++;
+ ResourceOwnerRememberBuffer(CurrentResourceOwner, buffer);
}
/*
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 845023590ea..53458cbbd62 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1194,6 +1194,13 @@ OpenTemporaryFile(bool interXact)
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
* one. If a given tablespace can't be found, we silently fall back to
* the database's default tablespace.
@@ -1229,9 +1236,8 @@ OpenTemporaryFile(bool interXact)
{
VfdCache[file].fdstate |= FD_XACT_TEMPORARY;
- ResourceOwnerEnlargeFiles(CurrentResourceOwner);
- ResourceOwnerRememberFile(CurrentResourceOwner, file);
VfdCache[file].resowner = CurrentResourceOwner;
+ ResourceOwnerRememberFile(CurrentResourceOwner, file);
/* ensure cleanup happens at eoxact */
have_xact_temporary_files = true;