Re: pgsql: Add putenv support for msvcrt from Visual Studio 2013

* Noah Misch wrote:

> On Tue, Apr 26, 2016 at 07:39:29PM +0200, Christian Ullrich
> wrote:

> > * Christian Ullrich wrote:

> Patch 1 looks good, except that it should be three patches:
>
> - cosmetic parts: change whitespace and s/0/NULL/
> - remove CloseHandle() call
> - probe for debug CRT modules, not just release CRT modules

Attached as 0001, 0002, 0003, in that order.

0004 is what used to be 0002, it disables caching of "DLL not
loaded" results.

> I recommend also moving the SetEnvironmentVariable() call before
> the putenv calls. That way, if a CRT loads while pgwin32_putenv()
> is executing, the newly-loaded CRT will always have the latest
> value.

Agreed, attached as 0005.

0006 was previously known as 0004, removing all caching.

> > Even with patch 0004, there is still a race condition between
> > the main thread and a theoretical additional thread created by
> > some other component that unloads some CRT between
> > GetProcAddress() and the _putenv() call, but that is hardly
> > fixable.
>
> I think you can fix it by abandoning GetModuleHandle() in favor
> of GetModuleHandleEx() + GetProcessAddress() + FreeLibrary().

Attached as 0007.

> > There is another possible fix, ugly as sin, if we really need
> > to keep the whole environment update machinery *and* cannot do
> > the full loop each time. Patch 0003 pins each CRT when we see
> > it for the first time.

This is now 0008.

Patch topology: master --- 1 .. 5 --- 6 --- 7
\
`- 8

> I prefer the simplicity of abandoning the cache (patch 4), if it
> performs decently. Would you compare the performance of patch 1,
> patches 1+2+3, and patches 1+2+4? This should measure the right
> thing (after substituting @libdir@ for your environment):

Tested with release builds; Core i7-6700K (quad/HT; 4000 MHz).
I did three runs each, and they were always within 0.5 % of each
other's run time.

- patch 1 (now 1-3): 24 μs/iteration (24 s for 1e6)
- patch 1+2+3 (now 1-5 + 8): 29 μs/iteration
- patch 1+2+4 (now 1-7): 30 μs/iteration

I also did a debug build with 1+2+4 that came in at 84 μs/iteration.

--
Christian

... now how do I get all the dangling debris out of the repo ...