From f609d074385e87688c93966e57e9229a889d5f95 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 30 Sep 2013 11:29:09 +0300 Subject: [PATCH] Fix snapshot leak if lo_open called on non-existent object. lo_open registers the currently active snapshot, and checks if the large object exists after that. Normally, snapshots registered by lo_open are unregistered at end of transaction when the lo descriptor is closed, but if we error out before the lo descriptor is added to the list of open descriptors, it is leaked. Fix by moving the snapshot registration to after checking if the large object exists. Reported by Pavel Stehule. Backpatch to 8.4. The snapshot registration system was introduced in 8.4, so prior versions are not affected (and not supported, anyway). --- src/backend/storage/large_object/inv_api.c | 44 +++++++++++++--------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c index b98110cd99..8cb34482bd 100644 --- a/src/backend/storage/large_object/inv_api.c +++ b/src/backend/storage/large_object/inv_api.c @@ -240,29 +240,18 @@ LargeObjectDesc * inv_open(Oid lobjId, int flags, MemoryContext mcxt) { LargeObjectDesc *retval; - - retval = (LargeObjectDesc *) MemoryContextAlloc(mcxt, - sizeof(LargeObjectDesc)); - - retval->id = lobjId; - retval->subid = GetCurrentSubTransactionId(); - retval->offset = 0; + Snapshot snapshot = NULL; + int descflags = 0; if (flags & INV_WRITE) { - retval->snapshot = SnapshotNow; - retval->flags = IFS_WRLOCK | IFS_RDLOCK; + snapshot = SnapshotNow; + descflags = IFS_WRLOCK | IFS_RDLOCK; } else if (flags & INV_READ) { - /* - * We must register the snapshot in TopTransaction's resowner, because - * it must stay alive until the LO is closed rather than until the - * current portal shuts down. - */ - retval->snapshot = RegisterSnapshotOnOwner(GetActiveSnapshot(), - TopTransactionResourceOwner); - retval->flags = IFS_RDLOCK; + snapshot = GetActiveSnapshot(); + descflags = IFS_RDLOCK; } else ereport(ERROR, @@ -271,11 +260,30 @@ inv_open(Oid lobjId, int flags, MemoryContext mcxt) flags))); /* Can't use LargeObjectExists here because it always uses SnapshotNow */ - if (!myLargeObjectExists(lobjId, retval->snapshot)) + if (!myLargeObjectExists(lobjId, snapshot)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("large object %u does not exist", lobjId))); + /* + * We must register the snapshot in TopTransaction's resowner, because + * it must stay alive until the LO is closed rather than until the + * current portal shuts down. Do this after checking that the LO exists, + * to avoid leaking the snapshot if an error is thrown. + */ + if (snapshot != SnapshotNow) + snapshot = RegisterSnapshotOnOwner(snapshot, + TopTransactionResourceOwner); + + /* All set, create a descriptor */ + retval = (LargeObjectDesc *) MemoryContextAlloc(mcxt, + sizeof(LargeObjectDesc)); + retval->id = lobjId; + retval->subid = GetCurrentSubTransactionId(); + retval->offset = 0; + retval->snapshot = snapshot; + retval->flags = descflags; + return retval; } -- 2.40.0