Re: BUG #15105: OpenTransientFile() should be paired with CloseTransientFile() rather than close()

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: bianpan2016@163.com
Cc: Michael Paquier <michael@paquier.xyz>, pgsql-bugs@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>
Date: 2018-04-10T22:40:43Z
Lists: pgsql-bugs

Attachments

I wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> On Fri, Mar 09, 2018 at 03:29:25AM +0000, PG Bug reporting form wrote:
>>> Details: The handler opened with OpenTransientFile() should be closed with
>>> CloseTransientFile(). However, in function dsm_impl_mmap(), on a certain
>>> path, the return value of OpenTransientFile() (at line 885) is passed to
>>> close() (at line 926). It is better to use CloseTransientFile() here, as
>>> done at line 909.

>> Good catch!  This is visibly a copy-paste error coming from
>> dsm_impl_posix().  As a patch it gives the attached.  I am adding also
>> Robert in CC for awareness.

Now that the commitfest crunch is over, I've checked and pushed this.

> Presumably, this would have been found sooner if AtEOXact_Files acted
> like most other end-of-xact cleanup functions and whined about leaked
> resources (in the commit case).  I wonder why it doesn't.

On closer inspection, such a cross-check wouldn't have helped find this
particular mistake, because it was in an error exit path that's likely
never been exercised by anybody, and certainly isn't hit in the regression
tests.  Still, I think it'd be a good thing to add, and hence propose the
attached patch.  (I've verified that the warning doesn't fire in "make
check-world".)

			regards, tom lane

Commits

  1. In AtEOXact_Files, complain if any files remain unclosed at commit.

  2. Fix incorrect close() call in dsm_impl_mmap().