Proj4 initialization crash on windows

[springmeyer]

We are seeing that processes that need to reproject data will crash on windows. This has likely be happening for some time (causing crashing for Mapbox Studio windows users). This may be a mapnik core problem in how Mapnik uses proj4. But so far it has only seemed to happen in Mapbox Studio (and therefore seems related to vector tiles). So, let's track this issue here for now.

[wilhelmberg]

Solved!
Non thread safe setlocale was used in pj_init_ctx of proj4.

See also:
OSGeo/PROJ#226

Patch:
https://github.com/mapbox/windows-builds/blob/master/patches/proj-4.8.0-setlocale.diff

[springmeyer]

awesome work tracking this down @BergWerkGIS!!!

[springmeyer]

For the record: I ran the node export.js at the latest commit https://github.com/BergWerkGIS/mapnik-proj4-crashes/commit/15b8f30fc227d01da27b3bce0dc42a61b0181f4b and using the default install ([email protected]) but did not encounter a crash. The entire export ran and completed (16 r/s). So I wondered if I needed to use a different locale. I set my system locale to "German(Austria)" but that also did not prompt a crash. So, I paused and have not tried to replicate with latest node-mapnik built from source.

[wilhelmberg]

@springmeyer
Did you just set the GUI to German, or did you also change the number format?
I have GUI in English, but number format is German, decimal symbol: , not ..

As the proj4 code in question sets

setlocale(LC_NUMERIC

I suppose, that's what causes the problem.

LC_NUMERIC: Affects the decimal-point character in formatted input/output operations
and string formatting functions, as well as non-monetary information returned by localeconv.

[wilhelmberg]

Considering performance. Allmost 1/2 of the time is spent in shape_io::shape_io:

Hot path within shape_io:

[springmeyer]

Did you just set the GUI to German, or did you also change the number format?

I did not change the number format. But now I did and I'm still not able to replicate a crash. Running node export.js against [email protected] gets to z15 no problem. Should I expect it to crash quickly or do I need to run it multiple times all the way to z16?.

Here are my new settings:

[springmeyer]

Considering performance, majority of time spent in shapefile reading makes sense. I also see ~ 16 tiles/s on OS X, so I should be able to figure out how to speed things up via OS X profiling.

[wilhelmberg]

Number Format is behind "Additional Settings"

[wilhelmberg]

Should crash quickly. Never got beyond level 10.

[springmeyer]

Number Format is behind "Additional Settings"

Confirmed my number format behind "Additional Settings" also lists , as the Decimal.

[springmeyer]

re-opening to remember to circle back here to further isolate and make sure I replicate after @BergWerkGIS is back next week.

[wilhelmberg]

tests with different proj4 x64 builds:

• [email protected]: stock version as distributed via npmjs (contains proj 4.8.0 patch)
• master: mapnik and node-mapnik master as of today
• proj 4.8.0 patch: commented setlocale
• proj 4.9.1 patch: replaced setlocale with _wsetlocale
• all machines with English GUI, but German keyboard layout and German number format

❌ -> crash
✅ -> no crash (didn't check validity of vtiles tough)

machine	[email protected]	master, proj 4.8.0 stock	master, proj 4.8.0 patch	master, proj 4.9.1 stock	master, proj 4.9.1 patch
EC2 (vanilla Server 2012)	✅	✅	✅	✅	✅
local W7	✅	❌	✅	❌	✅
local W8.1	✅	❌	✅	❌	✅
VM W7-1	✅	❌	✅	❌	✅
VM W7-2	✅	❌	✅	❌	✅

[wilhelmberg]

Summarizing above tests:

• tested with VS2014 CTP4, should repeat with VS2015 RC/RTM
• only tested with x64, should repeat tests with x86
• did not find the real reason, why crash is triggered on my local machines and not on EC2, tried
  • different locales
  • different number formats
  • different location
  • diffrent env vars: %LANG%, %LC_NUMERIC%
• I suggest we go with "replacing setlocale with _wsetlocale" instead of commenting, as the proj4 faq states: If a locale is in effect that modifies formatting of numbers, altering the role of commas and periods in numbers, then PROJ.4 will not work.

[wilhelmberg]

Test case is not crashing for me anymore, using current proj.4/master.

Recap:

• to init a projection proj4 needs to parse projection definition strings
• to do that correctly, it needs to know the locale, e.g. decimal delimiter . vs. ,
• locale was set to ensure atof and strod work as expected
• but setlocale is not thread safe -> crashes
• solution: locale safe variants pj_atof() and pj_strtod() were introduced and setlocale was removed

Next steps:
Move back to a release version as soon as it is available.