Re: Thread-safe nl_langinfo() and localeconv()

On 13/08/2024 08:45, Thomas Munro wrote:
> Hi,
> 
> Over on the discussion thread about remaining setlocale() work[1], I
> wrote out some long boring theories about $SUBJECT.  Here are some
> draft patches to try those theories out, and make a commitfest entry.
> nl_langinfo_l() is a trivial drop-in replacement, and
> pg_localeconv_r() has 4 different implementation strategies:
> 
> 1.  Windows, with ugly _configthreadlocale() and thread-local result.
> 2.  Glibc, with nice nl_langinfo_l() extensions.
> 3.  macOS/*BSD, with nice localeconv_l().
> 4.  Baseline POSIX: uselocale() + localeconv() + honking great lock.
> 
> In reality it'd just be Solaris running #4 (and AIX if it comes back).
> Whether they truly implement it as pessimally as the standard allows,
> who knows... you could drop the lock if you somehow knew that they
> returned a pointer to thread-local storage or a member of the locale_t
> object.

Patches 1 and 2 look good to me.

Patch 3 makes sense too, some comments on the details:

The #ifdefs and the LCONV_MEMBER stuff makes it a bit hard to follow 
what happens in each implementation strategy. I wonder if it would be 
more clear to duplicate more code.

There's a comment at the top of pg_locale.c ("!!! NOW HEAR THIS !!!") 
that needs to be removed or adjusted now.


>      * The POSIX standard explicitly says that it is undefined what happens if
>      * LC_MONETARY or LC_NUMERIC imply an encoding (codeset) different from
>      * that implied by LC_CTYPE.  In practice, all Unix-ish platforms seem to
>      * believe that localeconv() should return strings that are encoded in the
>      * codeset implied by the LC_MONETARY or LC_NUMERIC locale name.  Hence,
>      * once we have successfully collected the localeconv() results, we will
>      * convert them from that codeset to the desired server encoding.

The patch loses this comment, leaving just a much shorter comment in the 
WIN32 implementation. But it still seems like a relevant comment for the 
!WIN32 implementation too.


> This gets rid of some setlocale() calls and makes the returned value
> unclobberable with a defined lifetime.  The remaining call to
> setlocale() is only a query of the name of the current local (in a

typo: local -> locale

> multi-threaded future this would have to be changed, perhaps to use a
> per-database or per-backend locale_t instead of LC_GLOBAL_LOCALE).
> 
> All known non-Windows targets have nl_langinfo_l(), from POSIX 2018.

I think that's supposed to be POSIX 2008

-- 
Heikki Linnakangas
Neon (https://neon.tech)