Re: pgsql: Allow concurrent-safe open() and fopen() in frontend code for Wi

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Tue, Sep 18, 2018 at 09:11:43AM +0900, Michael Paquier wrote:
>> What I think I broke is that CreateFile ignores what _fmode uses, which
>> has caused the breakage, while calling directly open() or fopen() does
>> the work. There are also other things assuming that binary mode is
>> used, you can grep for "setmode" and see how miscinit.c or pg_basebackup
>> do the job.

> I have been playing with this stuff, and hacked the attached. Now, while
> TAP tests of initdb and pgbench are happy (I can actually see the past
> failure as well), pg_dump complains at authentication time when using
> plain-text mode when using databases with all ASCII characters. That's
> not something I expected first, but _get_fmode also influences
> operations like pipe(), which is something that pg_dump uses, and
> setmode is enforced to binary mode only when adapted.

> I am getting to wonder if what's present on HEAD represents actually the
> best deal we could get. Attached is the patch I used for reference.
> Thoughts?

Well, we have to do something. I have a report from EDB's packagers
that in 11beta4, "initdb --pwfile" is failing on Windows (ie, one can't
connect afterwards using the specified password). It seems nearly
certain to me that the reason is that the file is read with

FILE *pwf = fopen(pwfilename, "r");

and so the \r isn't getting stripped from what's used as the password.

So my opinion is that it's not negotiable that we have to restore
the previous behavior in this realm. I don't want to be running
around finding other cases one at a time, and I absolutely won't
hold still for putting an "#ifdef WIN32" around each such case.
(Even if we fixed all our own code, we'd still be breaking 3rd-party
code.)

What I don't understand yet is why your latest patch doesn't restore
the previous behavior. What exactly is still different?

In the meantime, we might be well advised to revert this patch in
v11 and just continue to work on the problem in HEAD. I see now
that this wasn't something to cram in during late beta ...

regards, tom lane