Re: reproduced elusive cygwin bug

Tom,
Michael,

Please post to pgsql-cygwin(at)postgresql(dot)org instead of sending private
email so that others can benefit and possibly help and to record this
issue in the archives too.

On Tue, Jan 15, 2002 at 03:38:56PM -0500, Tom Lane wrote:
> Michael Adler <adler(at)glimpser(dot)org> writes:
> >> Most curious. Needless to say, that test procedure shows no problems on
> >> Unix machines. Perhaps a cygwin bug?
>
> > Correct. In the first thread mentioned above, you and Jason discuss what
> > is probably happening. Windows doesn't let one process delete another's
> > file. For some reason, it fails less gracefully here.
>
> Hmm. Actually, the sequence of events appears to be:
>
> 1. Vacuum process deletes pg_internal.init (as it's supposed to).
>
> 2. next backend to start attempts to create a new pg_internal.init.
> It does this by creating a temporary file name, writing it, and then
> renaming it into place. (This is to ensure we don't leave broken files
> behind if two or more new backends try to do this at about the same
> time. On Unix, at least, rename() is guaranteed atomic.)

I can reproduce the problem with just the above two steps...

> BTW, I do not see what connection a temp table would have with this.
> AFAICS it wouldn't matter what you vacuumed. Does it really have to
> be a temp table?

..and I agree that temporary tables are not part of the root cause.

> It would appear that if the process that deleted pg_internal.init is
> still around, a rename to create a fresh pg_internal.init fails. I
> cannot understand why this should be; surely it's either a Windows bug
> or a cygwin bug?

Actually, the root cause is due to the following Windows "feature":

If one process has a file open (i.e, has a open handle), then other
processes will not be able to perform certain operations on this file.

Hence, until *all* handles open during the unlink() are closed, rename()
(and other operations) will fail.

So the problem is, in fact, even worse. Consider the following scenario:

1. A connects
2. B connects
3. C connects
4. B vacuums
5. D connects
6. B disconnects
7. D connects
8. A disconnects
9. C disconnects
10. D connects

After 3, the three backends associated with each client have open handles
to pg_internal.init (verified by the SysInternals Handle utility). After
4, pg_internal.init is successfully deleted (verified by Cygwin's strace).

During 5, rename() fails because the backends associated with A, B,
and C have open handles. During 7, rename() stills fail even though
the backend that performed the vacuum has exited (and closed its handle)
because of the other open handles. During 10, rename() succeeds because
*all* handles open during unlink() (i.e., DeleteFile()) have been closed.

> If it's true, however, you should be able to repeat
> the problem with a couple of trivial C programs,

See attached for such programs which can be built as follows:

$ g++ -o open open.cc
$ g++ -o unlink unlink.cc

The following is a sample run:

$ # window 1
$ >foo
open foo
Enter any character (and CF) to continue...

$ # window 2
$ unlink foo
Enter any character (and CF) to continue...

$ # window 3
$ # both open and unlink running
$ handle foo | fgrep foo
open.exe pid: 1784 C:\home\jtishler\tmp\pgsql\foo
unlink.exe pid: 2284 C:\home\jtishler\tmp\pgsql\foo
$ ls foo
ls: foo: Permission denied
$ # only open running
$ ls foo
ls: foo: Permission denied
$ # both open and unlink have exited
$ ls foo
ls: foo: No such file or directory
$ date >foo
$ cat foo
Wed Jan 16 09:03:18 2002

> which would be a fit setting for a bug report to the cygwin people.

IMO, this is not fixable in Cygwin. Unfortunately, I don't think
that this is fixable in PostgreSQL either -- unless it can close all
pg_internal.init file descriptors when necessary. Hopefully, I'm wrong...

Jason

Jason Tishler <jason(at)tishler(dot)net> writes:
>> It would appear that if the process that deleted pg_internal.init is
>> still around, a rename to create a fresh pg_internal.init fails. I
>> cannot understand why this should be; surely it's either a Windows bug
>> or a cygwin bug?

> Actually, the root cause is due to the following Windows "feature":

> If one process has a file open (i.e, has a open handle), then other
> processes will not be able to perform certain operations on this file.

> Hence, until *all* handles open during the unlink() are closed, rename()
> (and other operations) will fail.

Hmm. But pg_internal.init is not held open, or shouldn't be. It should
be read once and immediately closed during backend startup. Are we
leaking an open file descriptor someplace?

Based on your description, there would be a small window for the problem
to occur anyway, but the window seems to be much larger than it oughta
be.

regards, tom lane

I said:
> Hmm. But pg_internal.init is not held open, or shouldn't be. It should
> be read once and immediately closed during backend startup.

Well damn. init_irels() forgets to close the file. Will fix for 7.2.

Meanwhile, that still leaves us with a JDBC problem. Michael, will you
report that to the pgsql-jdbc list?

regards, tom lane

Tom,

On Wed, Jan 16, 2002 at 10:40:52AM -0500, Tom Lane wrote:
> Hmm. But pg_internal.init is not held open, or shouldn't be. It should
> be read once and immediately closed during backend startup. Are we
> leaking an open file descriptor someplace?

Yes. In src/backend/utils/cache/relcache.c:init_irels(), there is a
FileNameOpenFile() but no corresponding FileClose(). I will patch my
7.1.3 and see if this solves the problem.

Jason

On Wed, 16 Jan 2002, Tom Lane wrote:

> I said:
> > Hmm. But pg_internal.init is not held open, or shouldn't be. It
should
> > be read once and immediately closed during backend startup.
>
> Well damn. init_irels() forgets to close the file. Will fix for 7.2.
>
> Meanwhile, that still leaves us with a JDBC problem. Michael, will you
> report that to the pgsql-jdbc list?

Yes, I'll report as soon as I am granted access to that list. The
subscriber hasn't responded with a confirmation, however, after several
hours I started receiving the CYGWIN messages. I guess I'll shortly be on
th JDBC list.

It looks like the JDBC interfaces throws a nasty SQLexception when it
receives any NOTICE (or any message at that point which starts with "N").
I think that's a bug.

Mike Adler

Tom,

On Wed, Jan 16, 2002 at 11:02:06AM -0500, Tom Lane wrote:
> I said:
> > Hmm. But pg_internal.init is not held open, or shouldn't be. It should
> > be read once and immediately closed during backend startup.
>
> Well damn. init_irels() forgets to close the file. Will fix for 7.2.

The attached patch (against Cygwin PostgreSQL 7.1.3-1) solves this
problem. I will release 7.1.3-2 ASAP.

Jason

Jason Tishler <jason(at)tishler(dot)net> writes:
> The attached patch (against Cygwin PostgreSQL 7.1.3-1) solves this
> problem. I will release 7.1.3-2 ASAP.

I have committed a fix to CVS for 7.2, also.

regards, tom lane