`setenv("LANG", ...)` too late

[Floessie]

Alberto extended the MultiLangMgr to set the "LANG" environment variable in #4278. This is definitely the way to go, but doesn't fix the problem for me:

Version: 5.3-514-gac1238e7
Branch: dev
Commit: ac1238e7
Commit date: 2018-01-22
Compiler: cc 6.3.0
Processor: x86_64
System: Linux
Bit depth: 64 bits
Gtkmm: V3.22.0
Lensfun: V0.3.2.0
Build type: release
Build flags:  -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -Wall -Wno-unused-result -Wno-deprecated-declarations -O3 -DNDEBUG
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

If I place the setenv() in front of gtk_init() it's effective, but not after it.

Best, Flössie

[heckflosse]

@Floessie Great you found a solution. Why don't you just push it?

[Floessie]

@heckflosse No, that's no real solution, thus I didn't push it. The problem is, that MultiLangMgr depends on the options being ready, while the options depend on things that happen after gtk_init(). Just taking the options loading from init_rt() to the line before gtk_init() made it crash and I didn't have the time to dig deeper yesternight. I'll give it another try this week...

[heckflosse]

@Floessie Did you test whether it works using putenv() instead of setenv()? I had to use putenv() for windows builds, because setenv() is not available on msys2.

[Floessie]

@heckflosse I'll give it a try! 👍

[Beep6581]

Could be related: https://discuss.pixls.us/t/rawtherapee-5-3-524-and-gimp-2-9-9-on-ubuntu-studio-17-10/6546

[Floessie]

@heckflosse I've tried it without success (as I already expected). I hope the msys2 supplied putenv() is implemented better than what POSIX mandates.

Neither setenv() nor putenv() are thread-safe, so they have to be called as early as possible.

[heckflosse]

@Floessie it's putenv(const char *_EnvString)

[Floessie]

@heckflosse And the ownership? Always copied?

[heckflosse]

@Floessie don't know. Can't find the source. Just the header file :(

[Floessie]

I found a solution. 🎉

diff --git a/rtgui/main.cc b/rtgui/main.cc
index bd8f381c4..ca05fdef0 100644
--- a/rtgui/main.cc
+++ b/rtgui/main.cc
@@ -218,13 +218,6 @@ int processLineParams ( int argc, char **argv )

 bool init_rt()
 {
-    try {
-        Options::load();
-    } catch (Options::Error &e) {
-        std::cout << "ERROR: " << e.get_msg() << std::endl;
-        return false;
-    }
-
     extProgStore->init();
     SoundManager::init();

@@ -487,9 +480,6 @@ int main (int argc, char **argv)
     argv2 = "";

     Glib::init();  // called by Gtk::Main, but this may be important for thread handling, so we call it ourselves now
-    gdk_threads_set_lock_functions (G_CALLBACK (myGdkLockEnter), (G_CALLBACK (myGdkLockLeave)));
-    gdk_threads_init();
-    gtk_init (&argc, &argv);  // use the "--g-fatal-warnings" command line flag to make warnings fatal
     Gio::init ();

@@ -649,6 +639,17 @@ int main (int argc, char **argv)

     int ret = 0;

+    try {
+        Options::load();
+    } catch (Options::Error &e) {
+        std::cout << "ERROR: " << e.get_msg() << std::endl;
+        return false;
+    }
+
+    gdk_threads_set_lock_functions (G_CALLBACK (myGdkLockEnter), (G_CALLBACK (myGdkLockLeave)));
+    gdk_threads_init();
+    gtk_init (&argc, &argv);  // use the "--g-fatal-warnings" command line flag to make warnings fatal
+
     if (remote) {
         char *app_argv[2] = { const_cast<char *> (argv0.c_str()) };
         int app_argc = 1;

Still, the suffix problem should be fixed in multilangmgr.cc.

HTH, Flössie

[Beep6581]

This sounds like something we should have in 5.4, no?

[Floessie]

Uhm, the tought of having something so big (GTK init shifted) so late troubles me. But it's your decision.

We also need another patch for multilangmgr.cc that first reads LANG, remembers the suffix, and adds it back when setting LANG.

[heckflosse]

@Floessie

We also need another patch for multilangmgr.cc that first reads LANG, remembers the suffix, and adds it back when setting LANG.

diff --git a/rtgui/multilangmgr.cc b/rtgui/multilangmgr.cc
index b4896d69..a760de2e 100644
--- a/rtgui/multilangmgr.cc
+++ b/rtgui/multilangmgr.cc
@@ -115,6 +115,16 @@ const LocaleToLang localeToLang;
 void setGtkLanguage(const Glib::ustring &language)
 {
     auto l = localeToLang.getLocale(language);
+
+    if (l != "C") {
+        Glib::ustring language(getenv("LANG"));
+        if(!language.empty()) {
+            auto suffixPos = language.find_first_of(".");
+            if(suffixPos != Glib::ustring::npos) {
+                l += language.substr(suffixPos, Glib::ustring::npos);
+            }
+        }
+    }
 #ifdef WIN32
     putenv(("LANG=" + l).c_str());
 #else

[Floessie]

Exactly! 👍 I had no idea, if there are oddities with msys2, like the ones for putenv().

[heckflosse]

@Floessie I tested your patch on Win7. Works fine. 👍 to commit

[Floessie]

@heckflosse Ingo, I'll do so tonight. May I also include your patch, or will you push it?

[heckflosse]

@Floessie please include my patch

[heckflosse]

@Floessie damn

[Floessie]

@heckflosse Anyway, I think that's the right way to go. I'll push both patches to dev, so @Beep6581 can decide, if he cherry-picks them to release5.4 (no dash this time?). It could well be that MacOS doesn't like different locales in different threads...

[heckflosse]

@Floessie yes, absolutely :+1: to push both patches

[Floessie]

@heckflosse I think, I understand, what's going on. When I switch RT to a language I have no locale for, it prints:

(process:3250): Gtk-WARNING **: Locale not supported by C library.
    Using the fallback 'C' locale.

Maybe, that's the problem with @Benitoite's build, too. MacOS seems to be very picky about locales. He switched to German, and I'm almost sure, he has no de_DE locale installed. Check with locale -a.

[Floessie]

@heckflosse How about setlocale() and taking the return value into account?

[Floessie]

@heckflosse Yes, that could work...

[heckflosse]

@Floessie your try :)

[Floessie]

Here's a patch, that works under Linux:

diff --git a/rtgui/multilangmgr.cc b/rtgui/multilangmgr.cc
index 5ccac1566..3d3dddafa 100644
--- a/rtgui/multilangmgr.cc
+++ b/rtgui/multilangmgr.cc
@@ -19,6 +19,7 @@
 #include "multilangmgr.h"

 #include <fstream>
+#include <clocale>

 #ifdef WIN32
 #include <windows.h>
@@ -114,10 +115,21 @@ const LocaleToLang localeToLang;

 void setGtkLanguage(const Glib::ustring &language)
 {
+    const auto set_lang =
+        [](const std::string& lang)
+        {
+#ifdef WIN32
+            putenv(("LANG=" + lang).c_str());
+#else
+            setenv("LANG", lang.c_str(), true);
+#endif
+        };
+
+    const std::string env_lang = getenv("LANG");
+
     std::string lang = localeToLang.getLocale(language);

     if (lang != "C") {
-        const std::string env_lang = getenv("LANG");
         if (!env_lang.empty()) {
             const std::string::size_type suffix_pos = env_lang.find_first_of(".");
             if (suffix_pos != std::string::npos) {
@@ -126,11 +138,11 @@ void setGtkLanguage(const Glib::ustring &language)
         }
     }

-#ifdef WIN32
-    putenv(("LANG=" + lang).c_str());
-#else
-    setenv("LANG", lang.c_str(), true);
-#endif
+    set_lang(lang);
+
+    if (!std::setlocale(LC_ALL, "")) {
+        set_lang(env_lang);
+    }
 }

 }

[heckflosse]

@Floessie compiling...

[heckflosse]

@Floessie works fine on Win7 👍

[Benitoite]

@Floessie latest dev pull plus the patch makes the situation worse, now crashing on macOS English / United States with same error when switching to queue, with queued image in folder containing ö/ü.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libcairomm-1.0.1.dylib          0x000000010a47efe0 Cairo::Context::set_source(Cairo::RefPtr<Cairo::Surface> const&, double, double) + 16 (surface.h:364)
1   rawtherapee-bin                 0x00000001087262ab ThumbBrowserEntryBase::draw(Cairo::RefPtr<Cairo::Context>) + 699
2   rawtherapee-bin                 0x0000000108723081 ThumbBrowserBase::Internal::on_draw(Cairo::RefPtr<Cairo::Context> const&) + 369
3   libgtkmm-3.0.1.dylib            0x0000000109726f2f Gtk::Widget_Class::draw_callback(_GtkWidget*, _cairo*) + 175
4   libgtk-3.0.dylib                0x0000000109e1971c gtk_widget_draw_internal + 364 (gtkwidget.c:7027)
5   libgtk-3.0.dylib                0x0000000109c35e9a gtk_container_propagate_draw + 362 (gtkcontainer.c:3840)
6   libgtk-3.0.dylib                0x0000000109c36443 gtk_container_draw + 147 (gtkcontainer.c:3655)

[Benitoite]

Also, the issue reported on pixls was not switching to German language, but keeping English language and changing region to Germany. For instance a user wants currency and time reported in the German manner, but keeps the OS using English. de_DE is definitely in locale -a but of course not en_DE which is reported by macOS in the case of English(language) + Germany(region). Maybe en_US works because it’s in locale -a but not en_DE?

[TooWaBoo]

Latest version of RT crashes when using an other language than English.

[Floessie]

If I can also make it crash under Linux we've at least reached consistency. And consistency is king. 😁

de_DE is definitely in locale -a but of course not en_DE which is reported by macOS in the case of English(language) + Germany(region). Maybe en_US works because it’s in locale -a but not en_DE?

Oh my, Apple... en_DE denotes "Englisch as spoken in Germany", something like Lübke (beware the umlaut) spoke, or more recently Günther Öttinger (beware the double umlauts).

To me it seems, that calling setlocale() with an unknown locale leads to SEGVs under certain proprietary OSes. I don't think, we can solve that here, but I would be much surer, if I saw a debug backtrace.

@heckflosse Ingo, how shall we proceed?

[gaaned92]

latest version crashes as @TooWaBoo noticed

But both rawtherapee.exe and rawtherapee-debug.exe run ok under GDB! thus no backtrace available

[Floessie]

Inkscape on macOS has similar problems.

There is/was also a problem with locales using libstdc++ instead of libc++, but we're only using a C function.

@gaaned92 Is that due to 4a2b9468d140df6cb7169a55ac038bebfd951664 or one of the later commits?

[Floessie]

@Benitoite Is this interesting?

[Floessie]

Darktable nowadays does this, and used to do it like this.

[gaaned92]

@Floessie 4a2b946 ok b53e773 crash

[Beep6581]

Tested dev 5.3-612-gb53e773f, works fine in Linux.

$ locale -a
ar_DZ.utf8
ar_SA.utf8
C
de_CH.utf8
en_SE
en_SE.utf8
en_US
en_US.iso88591
en_US.utf8
it_IT.utf8
POSIX

LANG=en_SE.UTF-8 LC_ALL=en_SE.UTF-8 ../rawtherapee/rawtherapee

LANG=xx_SE.UTF-8 LC_ALL=en_SE.UTF-8 ../rawtherapee/rawtherapee

LANG=en_SE.UTF-8 LC_ALL=xx_SE.UTF-8 ../rawtherapee/rawtherapee
(process:3297): Gtk-WARNING **: Locale not supported by C library.
        Using the fallback 'C' locale.

LANG=xxx LC_ALL=xxx ../rawtherapee/rawtherapee
(process:3471): Gtk-WARNING **: Locale not supported by C library.
        Using the fallback 'C' locale.

And if you want to see fun things, try

LC_ALL=ar_SA.UTF-8 ../rawtherapee/rawtherapee

[Floessie]

@gaaned92 That's at least something. Thanks for testing.

@Beep6581 With the patch above even those warnings should be gone.

You set both LANG and LC_ALL, darktable sets LANG and LANGUAGE, we only set LANG internally. Does anyone know, what is really needed here?

[Floessie]

@heckflosse @Beep6581 Adding the suffix or even setting the LANG like we do is crap: Of course there is no fr or fr.utf8 locale, it should be fr_FR.utf8. The Gtk-WARNINGs indicate that, now we set it before starting gtk_init().

But even if we add another column here, we have to figure out why things fail so badly on setlocale() or later. As I cannot reproduce the crashes under Linux, I cannot help here.

[Beep6581]

Quotes from the setlocale man page:

A locale name is typically of the form language[_territory][.codeset][@modifier], where language is an ISO 639 language code, territory is an ISO 3166 country code, and codeset is a character set or encoding identifier like ISO-8859-1 or UTF-8

For glibc, first (regardless of category), the environment variable LC_ALL is inspected, next the environment variable with the same name as the category (see the table above), and finally the environment variable LANG. The first existing environment variable is used. If its value is not a valid locale specification, the locale is unchanged, and setlocale() returns NULL.

@gaaned92 @TooWaBoo @Benitoite how exactly do you reproduce the crash - what does locale -a say, what do you set the language, territory and codeset to, and which environment variable(s) do you use?

e.g. does this crash?

LC_ALL=does_not.Exist rawtherapee

[TooWaBoo]

how exactly do you reproduce the crash Change the language in RT to English - Restart RT - this works fine Change the language in RT to non English - Restart RT - crash

which environment variable(s) do you use? I didn't set any environment variable

e.g. does this crash? Setting LC_ALL as an environment variable doesn't change anything. It's still crashing

[Floessie]

@TooWaBoo

e.g. does this crash?

Setting LC_ALL as an environment variable doesn't change anything. It's still crashing

Please set RT to English, so it works without crashing, then prepend LC_ALL=does_not.Exist to the command line.

[TooWaBoo]

@Floessie I don't get it. btw. I'm on windows

[Floessie]

btw. I'm on windows

Yes, this is crucial, as we don't see those crashes on Linux.

I don't get it.

We want to know, if simply setting the environment variable LC_ALL to something odd crashes RT. Therefore, RT must be working before the test. You wrote:

Change the language in RT to English - Restart RT - this works fine

Please do so. Then start RT from the command line and set the environment variable LC_ALL to does_not.Exist, however this is done on Windows. Then start RT. If it crashes we have an indication, if not, then...

HTH, Flössie

[TooWaBoo]

Setting LC_ALL has no effect. RT runs fine when the language is set to English and crashes on non English.

[Beep6581]

@TooWaBoo how did you set LC_ALL?

[TooWaBoo]

[gaaned92]

I set langauage to french

• if I launch directly RT it crashes
• if I launch from a mingw64 shell it runs. no warning message

MINGW64 shell environment variable

ACLOCAL_PATH=/mingw64/share/aclocal:/usr/share/aclocal
ALLUSERSPROFILE='C:\ProgramData'
APPDATA='C:\Users\Andre\AppData\Roaming'
BASH=/usr/bin/bash
BASHOPTS=cmdhist:complete_fullquote:expand_aliases:extglob:extquote:force_fignore:interactive_comments:login_shell:progcomp:promptvars:sourcepath
BASH_ALIASES=()
BASH_ARGC=()
BASH_ARGV=()
BASH_CMDS=()
BASH_COMPLETION_COMPAT_DIR=/etc/bash_completion.d
BASH_LINENO=()
BASH_SOURCE=()
BASH_VERSINFO=([0]="4" [1]="4" [2]="12" [3]="1" [4]="release" [5]="x86_64-pc-msys")
BASH_VERSION='4.4.12(1)-release'
BD=64
BR=dev
BRANCH=dev
CAMLIBS='C:\Program Files\darktable\lib\libgphoto2\2.5.16'
COLUMNS=80
COMMITSafter=612
COMMONPROGRAMFILES='C:\Program Files\Common Files'
COMPUTERNAME=C1
COMSPEC='C:\WINDOWS\system32\cmd.exe'
CONFIG_SITE=/mingw64/etc/config.site
CONTITLE='MinGW x64'
CommonProgramW6432='C:\Program Files\Common Files'
DASHLANE_DLL_DIR='C:\Users\Andre\AppData\Roaming\Dashlane\5.6.0.15520\bin\Firefox_Extension\{442718d9-475e-452a-b3e1-fb1ee16b8e9f}\components;C:\Users\Andre\AppData\Roaming\Dashlane\5.6.0.15520\ucrt'
DIRSTACK=()
DokanLibrary1='C:\Program Files\Dokan\Dokan Library-1.0.0\'
EUID=197610
FPS_BROWSER_APP_PROFILE_STRING='Internet Explorer'
FPS_BROWSER_USER_PROFILE_STRING=Default
GIT_TAG=$'3.0A1\n3.0A2\n3.0B1\n3.1.1\n4.0.0\n4.0.1\n4.0.10\n4.0.11\n4.0.12\n4.0.2\n4.0.3\n4.0.4\n4.0.5\n4.0.6\n4.0.7\n4.0.8\n4.0.9\n4.1\n4.2\n5.0-gtk2\n5.0-gtk3\n5.0-r1-gtk2\n5.0-r1-gtk3\n5.1\n5.1-rc1\n5.2\n5.3\n5.3-rc1\nDev-3.0\nDev-3.1\nDev-3.1m1\nDev-3.1m2\nDev-3.1m3\nDev-3.1m4\nDev-3.1m5\nDev-3.1m6\nDev-Darkframe\nDev-Defloat'
GROUPS=()
HISTFILE=/home/Andre/.bash_history
HISTFILESIZE=500
HISTSIZE=500
HOME=/home/Andre
HOMEDRIVE=C:
HOMEPATH='\Users\Andre'
HOSTNAME=C1
HOSTTYPE=x86_64
IFS=$' \t\n'
INFOPATH=/usr/local/info:/usr/share/info:/usr/info:/share/info
IOLIBS='C:\Program Files\darktable\lib\libgphoto2_port\0.12.0'
LANG=fr_FR.UTF-8
LD_LIBRARY_PATH=/home/Andre/instMINGW64/lib:/c/msys64/MINGW64/lib:
LINES=24
LOCALAPPDATA='C:\Users\Andre\AppData\Local'
LOGONSERVER='\\C1'
MACHTYPE=x86_64-pc-msys
MAGICK_HOME='C:\Program Files\darktable\lib\GraphicsMagick-1.3.25\modules-Q8\coders'
MAILCHECK=60
MAKEFLAGS=-j8
MANPATH=/mingw64/share/man:/usr/local/man:/usr/share/man:/usr/man:/share/man
MINGW_CHOST=x86_64-w64-mingw32
MINGW_MOUNT_POINT=/mingw64
MINGW_PACKAGE_PREFIX=mingw-w64-x86_64
MINGW_PREFIX=/mingw64
MOZ_PLUGIN_PATH='C:\Program Files\Tracker Software\PDF Viewer\Win32'
MSYS2_PATH=/usr/local/bin:/usr/bin:/bin
MSYSCON=mintty.exe
MSYSTEM=MINGW64
MSYSTEM_CARCH=x86_64
MSYSTEM_CHOST=x86_64-w64-mingw32
MSYSTEM_PREFIX=/mingw64
NUMBER_OF_PROCESSORS=8
OLDPWD=/d/rawtherapee/rtsource
OPTERR=1
OPTIND=1
ORIGINAL_PATH=/c/Windows/System32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/
ORIGINAL_TEMP=/c/Users/Andre/AppData/Local/Temp
ORIGINAL_TMP=/c/Users/Andre/AppData/Local/Temp
OS=Windows_NT
OSTYPE=msys
OneDrive='C:\Users\Andre\OneDrive'
PATH=/home/Andre/instMINGW64/bin:/mingw64/bin:/usr/local/bin:/usr/bin:/bin:/c/Windows/System32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl:/y/rtsource:/home/Andre
PATHEXT='.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC'
PIPESTATUS=([0]="0")
PKG_CONFIG_PATH=/home/Andre/instMINGW64/lib/pkgconfig:/mingw64/lib/pkgconfig:/mingw64/share/pkgconfig
PPID=13108
PREFIX=/home/Andre/instMINGW64
PRINTER='HP Deskjet 3070 B611 series (réseau)'
PROCESSOR_ARCHITECTURE=AMD64
PROCESSOR_IDENTIFIER='Intel64 Family 6 Model 94 Stepping 3, GenuineIntel'
PROCESSOR_LEVEL=6
PROCESSOR_REVISION=5e03
PROGRAMFILES='C:\Program Files'
PROMPT='$P$G'
PS1='\[\e]0;\w\a\]\n\[\e[32m\]\u@\h \[\e[35m\]$MSYSTEM\[\e[0m\] \[\e[33m\]\w\[\e[0m\]\n\$ '
PS2='> '
PS4='+ '
PSModulePath='C:\Program Files\WindowsPowerShell\Modules;C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules'
PUBLIC='C:\Users\Public'
PWD=/d/rawtherapee/rtinstall/devrelease64
ProgramData='C:\ProgramData'
ProgramW6432='C:\Program Files'
SESSIONNAME=Console
SHELL=/usr/bin/bash
SHELLOPTS=braceexpand:emacs:hashall:histexpand:history:interactive-comments:monitor
SHLVL=1
STABLE=dev
SYSCONFDIR=/etc
SYSTEMDRIVE=C:
SYSTEMROOT='C:\WINDOWS'
TEMP=/tmp
TERM=xterm
TMP=/tmp
TZ=Europe/Paris
UID=197610
USER=Andre
USERDOMAIN=C1
USERDOMAIN_ROAMINGPROFILE=C1
USERNAME=Andre
USERPROFILE='C:\Users\Andre'
VerAfter=5.3-612-gb53e773fb
VerBefore=5.3-611-g4a2b9468d
WD='C:\msys64\usr\bin\'
WINDIR='C:\WINDOWS'
WIN_ROOT=/c/Windows
_=./rawtherapee
_backup_glob='@(#*#|*@(~|.@(bak|orig|rej|swp|dpkg*|rpm@(orig|new|save))))'
lastbuild=RawTherapee_dev_5.3-611-g4a2b9468d_WinVista_64.zip
postinst=/etc/post-install/08-xml-catalog.post
temp='C:\Users\Andre\AppData\Local\Temp'
tmp='C:\Users\Andre\AppData\Local\Temp'

$ LC_ALL=does_not.Exist
bash: avertissement :setlocale : LC_ALL : impossible de changer le paramètre de langue (does_not.Exist)

[gaaned92]

locale -a output

[TooWaBoo]

Ahhhh, when you Linux guys talk about command line you mean Mingw and not Windows cmd. ;-) Running RT from Mingw64 command line works fine in all languages.