On Sat, Aug 16, 2025 at 12:50 AM Jingtang Zhang <mrdrivingduck@gmail.com> wrote:
> Back to v17, commit 21d9c3ee gave SMgrRelation a well-defined lifetime, and
> smgrclose nolonger removes SMgrRelation object from the hashtable, leaving
> the work to smgrdestroyall. But I find a place that relies on the removing
> behavior previously, but is still calling smgrclose.
Thanks for the report! Replying here rather than the pgsql-bugs
thread because your patch is here.
> Startup process of standby will redo table dropping with DropRelationFiles,
> using smgrdounlinkall to drop buffers and unlink physical files, and then
> uses smgrclose to destroy the SMgrRelation object. I think it should use
> smgrdestroy here, or the object memory will be leaked.
DropRelationFiles() is also called by FinishPreparedTransaction(). At
first I thought that might be a problem too, but looking a bit more
closely and trying it out... if a prepared transaction dropped a
table, then it called RelationDropStorage(), RelationCloseSmgr(),
smgrunpin(), and I can't immediately think of any way to repin it
while the relation is locked, so you can't break the assertion about
that in smgrdestroy(), where we make sure there are no Relation
objects with dangling references. It's already left unpinned or never
pinned and later destroyed in both DROP; PREPARE TRANSACTION; ...
COMMIT PREPARED; and CREATE; PREPARE TRANSACTION; ... ROLLBACK
PREPARED sequences, with different details.
Now I'm left wondering if two-phase commit should do this explicitly
or not. For the isRedo case it seems clear, it was the intention of
21d9c3ee to destroy it on commit/abort, which must be here I think.
The following description from the commit message *probably* also fits
the two-phase case, even though it already doesn't leak without it.
It would be good to get a comment explaining the new smgrdestroy()
call, and point to where our tests exercise all relevant cases. Hmm,
it's not immediately obvious how to introspect the cached state to
verify that the memory isn't leaked.
Guarantee that the object won't be destroyed until the end of the
current transaction, or in recovery, the commit/abort record that
destroys the underlying storage.
