save as Synfig (sif) crashes if the file contains non-<path> elements (trunk only)

[Alvin Penner]

in order to run Synfig output successfully in trunk it is first necessary to temporarily patch Bug 1663362. This can be done using comment 5 by jazzynico:
https://bugs.launchpad.net/inkscape/+bug/1663362/comments/5
a diff file showing the change is attached here.

[Alvin Penner (apenner)]

now do the test in trunk:
- Windows 10, Inkscape 0.92+devel 15500
- load the attached file synfig_test.svg
- save as Synfig (sif)
- get popup crash message attached

[Alvin Penner (apenner)]

- not reproduced if the Ellipse in the drawing is first deleted before saving
- not reproduced on Windows 10, Inkscape 0.92.0 r15299

I believe this is essentially the same as Bug 1663585 and also Bug 1662531
https://bugs.launchpad.net/inkscape/+bug/1662531/comments/8
but I decided to report it separately because Synfig is in the standard Inkscape build, not an external add-on. All three crashes have one thing in common, they all use Inkscape verbs from inside Python with a Popen command.

[su_v (suv-lp)]

Crash not reproduced with lp:inkscape r15500 on Ubuntu 14.04.5 LTS.

[Alvin Penner (apenner)]

- on Windows 10, with the temporary patch from comment 1 applied:
- crash not reproduced in trunk rev 15443
 - crash reproduced on Windows in trunk rev 15444
 - appears to have been introduced in:
 merge lp:~inkscape.dev/inkscape/doc_rotate
http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/15444

[Alvin Penner (apenner)]

attempting to perform a backtrace with rev 15444 and synfig patched as in comment 1, I get the result.
First the attached dialog

[Alvin Penner (apenner)]

followed by the popup message:
File C:\Windows\Temp\synfig_test_15444_5.sif could not be saved.

with the backtrace attached:

[su_v (suv-lp)]

Proposed modification for Synfig output to avoid the crash on Windows with 64bit trunk builds was attached to (otherwise not directly related) bug #1663362:
https://bugs.launchpad.net/inkscape/+bug/1663362/comments/8

(diff attached for easier review of the changes)

[su_v (suv-lp)]

As mentioned in comments of bug #1662531, maybe the proposed change to force the locale (via $LC_ALL) for spawned inkscape processes to fallback 'C' could be limited to affected platforms.

[Hachmann (marenhachmann)]

Is the contents of comment https://bugs.launchpad.net/inkscape/+bug/1662531/comments/16 wrong info?

[su_v (suv-lp)]

On 2017-02-18 17:56 (+0100), Hachmann wrote:
> Is the contents of comment
> https://bugs.launchpad.net/inkscape/+bug/1662531/comments/16 wrong info?

That comment refers to a modified version of the script from comment 9.

[Alvin Penner (apenner)]

yes, in retrospect I probably should not have posted that code, since it interferes with the normal sequence of events. A much less intrusive way of doing the monitoring is to use code like this:

f = p.communicate()
err.close()
fout = open('\Windows\Temp\save_as_err.txt', 'wt')
fout.write("\nstdout = " + f[0])
fout.write("\nstderr = " + f[1] + "\n")
fout.close()

which allows you to see both the normal and the error output.

[su_v (suv-lp)]

Latest findings:
https://bugs.launchpad.net/inkscape/+bug/1662531/comments/64

[su_v (suv-lp)]

Alternative patch to the one referred to in prior comment:
For fallback "C", check whether current_locale is empty string or None.

[su_v (suv-lp)]

Patch deleted - not a solution (I was wrong).

[su_v (suv-lp)]

Another update:
* The crash of core inkscape is now tracked in bug #1666314.

* A workaround to prevent the known trigger of the crash is described by Alvin in bug #1662531:
https://bugs.launchpad.net/inkscape/+bug/1662531/comments/69

* The question why LANG is set at all in inkex.localize() on Windows is still open (bug #1666108):
https://bugs.launchpad.net/inkscape/+bug/1666108/comments/1

* Proposed changes for inkex.py:
Unless LANG is needed for translations of messages returned by extension scripts, a solution to prevent the crash on Windows of inkscape (trunk) instances with GUI spawned via Popen() from extension scripts could be to remove line 57 in share/extensions/inkex.py (or to comment the line out, as in attached proposed diff).

* Explanation:
AFAICT default environments on Windows do not define %LANG%. If the variable is defined nevertheless to anything other than C or POSIX, then the initial instance already crashes on launch (bug #1666314). There is no reason to forcefully unset LANG on Windows in inkex.localize() because if it is set, affected inkscape builds (currently only trunk) already crash before any extension can be run.

Whether or not, and if to what extent, the environment variables LANG and LC_ALL of the parent process on Windows have an effect on inkscape for localization IMHO needs to be addressed in Inkscape core first: currently, these environment variables do affect at least the UI language (and possibly other aspects of localization like the character used for the decimal point) of the main application, even if rarely used on Windows.

[su_v (suv-lp)]

Not reproduced on Ubuntu 14.04.5 LTS with 0.92.x and trunk.

On Windows 10:
- not reproduced with Inkscape 0.92.1 (32bit, 64bit, 7z)
- reproduced with lp:inkscape r15537
  (see bug #1666314 for backtrace of core inkscape)

Proposed diff for inkex.py (comment 16) tested successfully on Windows 10 with lp:inkscape r15537.

[Alvin Penner (apenner)]

confirmed fixed in Inkscape 0.92+devel 15583

most likely fixed in rev 15572