win_flex.exe (and likely win_bison.exe) isn't concurrency safe

Started by Andres Freundover 3 years ago4 messages
#1Andres Freund
andres@anarazel.de

Hi,

building PG with meson on windows I occasionally got weird errors around
scanners. Sometimes scanner generation would fail with

win_flex.exe: error deleting file C:\Users\myadmin\AppData\Local\Temp\~flex_out_main_2

sometimes the generated scanner would just be corrupted.

I was confused by only hitting this in local VM, not in CI, but after finally
hunting it down it made more sense:
https://github.com/lexxmark/winflexbison/issues/86

https://github.com/lexxmark/winflexbison/blob/master/flex/src/main.c#L1051

It uses a temporary file name without any concurrency protection. Looks like
windows' _tempnam is pretty darn awful and returns a predictable name as long
as no conflicting file exists.

Our documentation doesn't point to winflexbison, but recommends using
flex/bison from msys. But I've certainly read about others using winflexbison,
e.g. [1]/messages/by-id/ae44b04a-2087-fa9d-c6b1-b1dcbbacf4ae@dunslane.net [2]/messages/by-id/CAGRY4nxJNqEjr1NtdB8=dcOwwsWTqQfykyvgp1i_udCtw--BkQ@mail.gmail.com. The flex/bison in 'chocolatey', which I've also seen referenced,
is afaics winflexbison.

Afaict the issue also exists in our traditional windows build - but I've not
seen anybody report this as an issue.

I started this thread to document the issue, in case developers using visual
studio are hitting this today.

It looks like a similar issue exists for the windows bison port:
https://github.com/lexxmark/winflexbison/blob/b2a94ad5fd82cf4a54690a879e14ff9511b77273/bison/src/output.c#L816

I've not observed that failure, presumably because the window for it is much
shorter.

For the meson build it is trivial to "address" this by setting FLEX_TMP_DIR to
a private directory (which we already need to deal with lex.backup), something
similar could be done for src/tools/msvc.

Unless we think it'd be better to just refuse to work with winflexbison?

Greetings,

Andres Freund

[1]: /messages/by-id/ae44b04a-2087-fa9d-c6b1-b1dcbbacf4ae@dunslane.net
[2]: /messages/by-id/CAGRY4nxJNqEjr1NtdB8=dcOwwsWTqQfykyvgp1i_udCtw--BkQ@mail.gmail.com

#2Amit Kapila
amit.kapila16@gmail.com
In reply to: Andres Freund (#1)
Re: win_flex.exe (and likely win_bison.exe) isn't concurrency safe

On Sat, Sep 3, 2022 at 4:13 AM Andres Freund <andres@anarazel.de> wrote:

building PG with meson on windows I occasionally got weird errors around
scanners. Sometimes scanner generation would fail with

win_flex.exe: error deleting file C:\Users\myadmin\AppData\Local\Temp\~flex_out_main_2

sometimes the generated scanner would just be corrupted.

I was confused by only hitting this in local VM, not in CI, but after finally
hunting it down it made more sense:
https://github.com/lexxmark/winflexbison/issues/86

https://github.com/lexxmark/winflexbison/blob/master/flex/src/main.c#L1051

It uses a temporary file name without any concurrency protection. Looks like
windows' _tempnam is pretty darn awful and returns a predictable name as long
as no conflicting file exists.

Our documentation doesn't point to winflexbison, but recommends using
flex/bison from msys. But I've certainly read about others using winflexbison,
e.g. [1] [2]. The flex/bison in 'chocolatey', which I've also seen referenced,
is afaics winflexbison.

Afaict the issue also exists in our traditional windows build - but I've not
seen anybody report this as an issue.

I started this thread to document the issue, in case developers using visual
studio are hitting this today.

I regularly use visual studio for the development work but never hit
this issue probably because I am using flex/bison from msys.

It looks like a similar issue exists for the windows bison port:
https://github.com/lexxmark/winflexbison/blob/b2a94ad5fd82cf4a54690a879e14ff9511b77273/bison/src/output.c#L816

I've not observed that failure, presumably because the window for it is much
shorter.

For the meson build it is trivial to "address" this by setting FLEX_TMP_DIR to
a private directory (which we already need to deal with lex.backup), something
similar could be done for src/tools/msvc.

Unless we think it'd be better to just refuse to work with winflexbison?

Personally, I have never used this but I feel it would be good to keep
it working especially if others are using it.

--
With Regards,
Amit Kapila.

#3Julien Rouhaud
rjuju123@gmail.com
In reply to: Amit Kapila (#2)
Re: win_flex.exe (and likely win_bison.exe) isn't concurrency safe

On Sat, Sep 03, 2022 at 10:33:43AM +0530, Amit Kapila wrote:

On Sat, Sep 3, 2022 at 4:13 AM Andres Freund <andres@anarazel.de> wrote:

For the meson build it is trivial to "address" this by setting FLEX_TMP_DIR to
a private directory (which we already need to deal with lex.backup), something
similar could be done for src/tools/msvc.

Unless we think it'd be better to just refuse to work with winflexbison?

Personally, I have never used this but I feel it would be good to keep
it working especially if others are using it.

My windows environments rely on winflexbison so I'd prefer if we keep the
compatibility (or improve it). I personally never hit that problem, but I
don't work much on Windows either.

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#1)
Re: win_flex.exe (and likely win_bison.exe) isn't concurrency safe

On 2022-09-02 Fr 18:43, Andres Freund wrote:

Hi,

building PG with meson on windows I occasionally got weird errors around
scanners. Sometimes scanner generation would fail with

win_flex.exe: error deleting file C:\Users\myadmin\AppData\Local\Temp\~flex_out_main_2

sometimes the generated scanner would just be corrupted.

I was confused by only hitting this in local VM, not in CI, but after finally
hunting it down it made more sense:
https://github.com/lexxmark/winflexbison/issues/86

https://github.com/lexxmark/winflexbison/blob/master/flex/src/main.c#L1051

It uses a temporary file name without any concurrency protection. Looks like
windows' _tempnam is pretty darn awful and returns a predictable name as long
as no conflicting file exists.

Our documentation doesn't point to winflexbison, but recommends using
flex/bison from msys. But I've certainly read about others using winflexbison,
e.g. [1] [2]. The flex/bison in 'chocolatey', which I've also seen referenced,
is afaics winflexbison.

Afaict the issue also exists in our traditional windows build - but I've not
seen anybody report this as an issue.

I started this thread to document the issue, in case developers using visual
studio are hitting this today.

It looks like a similar issue exists for the windows bison port:
https://github.com/lexxmark/winflexbison/blob/b2a94ad5fd82cf4a54690a879e14ff9511b77273/bison/src/output.c#L816

I've not observed that failure, presumably because the window for it is much
shorter.

For the meson build it is trivial to "address" this by setting FLEX_TMP_DIR to
a private directory (which we already need to deal with lex.backup), something
similar could be done for src/tools/msvc.

Unless we think it'd be better to just refuse to work with winflexbison?

No, I think your workaround is better if it works. I don't want to
require installation of msys to build with MSVC if that's avoidable.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com