Crash on launch if %LANG% not C or POSIX (Windows, trunk)

Bug #1666314 reported by su_v on 2017-02-20
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Undecided
Patrick Storz

Bug Description

Inkscape trunk crashes on launch on Windows if the current environment defines LANG to any (valid) locale but C or POSIX.

Steps to reproduce:
In Command Prompt window, change to inkscape's install folder:

C:\path\to\inkscape>set LANG=C
C:\path\to\inkscape>inkscape

--> inkscape launches as expected

C:\path\to\inkscape>set LANG=POSIX
C:\path\to\inkscape>inkscape

--> inkscape launches as expected

C:\path\to\inkscape>set LANG=en_US
C:\path\to\inkscape>inkscape

--> inkscape crashes on launch

C:\path\to\inkscape>set LANG=de_CH
C:\path\to\inkscape>inkscape

--> inkscape crashes on launch

Console message (without gdb):
terminate called after throwing an instance of 'std::runtime_error'
  what(): locale::facet::_S_create_c_locale name not valid

Notes:
* The crash is for example exposed if inkscape is launched with GUI (e.g. for verbs) via python-based extensions which use a subclass of inkex.Effect() (bug #1662531 , bug #1663697, bug #1663585), because inkex.localize() sets LANG in os.environ on Windows (bug #1666108).
* Not reproduced with Inkscape 0.91, 0.92.0, 0.92.1 (32bit, 64bit, installed from 7z to custom location): no crash on launch, inkscape sets the GUI language according to LANG as defined in the environment.
* Based on earlier bisecting by Alvin Penner, this regression seems to have been exposed with the merge of the doc-rotate branch (lp:inkscape r15444):
http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/15444

Reproduced with Inkscape 0.92+devel 15534 on Windows 10 (local build).

su_v (suv-lp) wrote :
su_v (suv-lp) on 2017-02-20
description: updated
su_v (suv-lp) wrote :

Just came across this recent message on one of GCC's mailing lists (no replies yet):
https://gcc.gnu.org/ml/gcc-help/2017-02/msg00080.html

Similarly, there is a known issue with libstdc++ on (older) versions of Mac OS X (before the switch to clang and libc++ as default): C++ locale only supports C, POSIX
https://mail.gnome.org/archives/gtkmm-list/2010-April/msg00030.html

Alvin Penner (apenner) wrote :

crashes confirmed on Windows 10, Inkscape 0.92+devel 15524 custom

just for curiosity do you get a crash if you set LANG = null
using a command like SET LANG =
?

Changed in inkscape:
status: New → Confirmed
Patrick Storz (ede123) wrote :

Confirmed with Inkscape 0.92+devel 15530 (devlibs 64) and 0.92+devel 15513 (MSYS2 build)

On 2017-02-20 22:09 (+0100), Alvin Penner wrote:
> just for curiosity do you get a crash if you set LANG = null
> using a command like SET LANG =

No crash with 'set LANG=' (in bash, the same command would be 'unset
LANG'). If LANG is unset, then there is no crash (presumably, that's the
default situation on Windows).

Patrick Storz (ede123) wrote :

Crash is in function "sp_dtw_rotation_output()" in "src/widgets/desktop-widget.cpp".

Alvin's bisecting was correct, specifically it was introduced in
http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/15142.1.24

Subscribing Jabier, as he probably knows what he wanted to achieve with this specific commit...

I hope we can do without locale conversions at this point. Shouldn't be necessary thanks to the UTF8-nature of GTK?

Patrick Storz (ede123) wrote :

Reverting the commit works just fine for me (German locale).

<off-bug-topic>
To be honest I also don't understand the conditional introduced in the linked commit:
If (new_scrolled == new_typed) then we can just as well set "*new_val = new_typed;" instead of "*new_val = new_scrolled;". No conditional needed there...
</off-bug-topic>

Patrick Storz (ede123) wrote :

Ah, OK, problem is that without the code I can't set a value of e.g. "10.5" as my system expects a comma as decimal separator (i.e. "10,5").

@Jabier: Couldn't we implement the current GtkSpinButton as "Inkscape::UI::Widget::SpinButton()"? It would take care of all these issues (and even adding functionality like allowing to enter simple arithmetic expressions).

Patrick Storz (ede123) wrote :

It seems that in fact the "std::locale()" is simply not supported by gcc, see [1]. It might work on *nix [2] but other OSs are out of luck.

As you can see in [1] the function "locale::facet::_S_create_c_locale()" throws for all values except "C".

[1] https://github.com/gcc-mirror/gcc/blob/b7937c548fe1cbe1ea562a2ba6f8c11d20691317/libstdc%2B%2B-v3/config/locale/generic/c_locale.cc#L220
[2] https://github.com/gcc-mirror/gcc/blob/HEAD/libstdc%2B%2B-v3/config/locale/gnu/c_locale.cc#L132

Patrick Storz (ede123) wrote :

Suggested fix for this issue (not pretty but it works).

@su_v: Did I use the correct degree symbol?

Jabiertxof (jabiertxof) wrote :

Thanks Eduard. For what is this part of the patch?

gchar *comma = g_strstr_len (b, -1, ",");
    if (comma) {
        *comma = '.';
     }

Patrick Storz (ede123) wrote :

It replaces comma with dot (e.g. if you enter "1,5" it's converted to "1.5").

Then the locale is set to "C" (which is using dots as decimal separators) and the value converted to a double which is then used to update the value of the SpinButton.

This way it does not matter what locale Inkscape is using, one can always use both - commas and dots - as decimal separator when typing a value.

Jabiertxof (jabiertxof) wrote :

I dont know I cann substitute in this way. Cool. comma coulden't be free?

Patrick Storz (ede123) wrote :

It should be freed from what I understand: "comma" points to a single gchar in the gchar-array "b". When b is freed, "comma" should be freed, too.

Jabiertxof (jabiertxof) wrote :

Thanks Edward. If finaly apply the patch to trunk ping me to apply also in measure-line-lpe

Patrick Storz (ede123) wrote :
Changed in inkscape:
assignee: nobody → Eduard Braun (eduard-braun2)
status: Confirmed → Fix Committed
Patrick Storz (ede123) wrote :

@Jabiertxof: Here's a patch we could apply against "lpe-measure-line.cpp" to avoid the occurrences of "std::locale("")".

It'd be nicer to be able to use std:: functions and std::strings, but I wasn't able to find a way to achieve locale-dependent string formatting with the incomplete state libstdc++ is in.

jazzynico (jazzynico) on 2017-03-07
Changed in inkscape:
status: Fix Committed → Fix Released
Jabiertxof (jabiertxof) wrote :

Thanks for fixing it!

Jabiertxof (jabiertxof) wrote :

And thanks for the measure line patch! I try tonight and commit

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers