Page MenuHome

Fix T88877: 2.93: Crash on recent OSX with a non-English locale.
ClosedPublic

Authored by Bastien Montagne (mont29) on Oct 28 2021, 10:16 AM.

Details

Summary

Looks like OSX changed the default format of its locale, which is not
valid anymore for gettext/boost::locale.

Solution based on investigations and patch by Kieun Mun (@KIEUN MUN (kieuns)), many
thanks.

Also add an exception catcher on std::runtime_error in
bl_locale_set(), since in OSX catching the ancestor std::exception
does not work with boost::locale::conv::conversion_error and the like
for some reasons.


Note to reviewers: This is a slightly tweked version of the patch by @KIEUN MUN (kieuns) posted
in a comment of T88877.

I cannot test this myself, we need to ensure that it both fixes the issue on the new OSX
version, and that it still works on the older ones.

Diff Detail

Repository
rB Blender
Branch
T88877 (branched from master)
Build Status
Buildable 18309
Build 18309: arc lint + arc unit

Event Timeline

Bastien Montagne (mont29) requested review of this revision.Oct 28 2021, 10:16 AM
Bastien Montagne (mont29) created this revision.

Suggest catching the boost conversion_error exception also in bl_locale_set so that older systems won't crash if this code breaks them.

I have not been able to redo the crash ever since T88877#1187007, despite hardcoding en_IN or even ko-Kore_KR in the code. Would have to rely on @KIEUN MUN (kieuns) for testing.

Just FYI if you want to make this OS version dependent [1], that can be done.
[1] https://developer.apple.com/documentation/foundation/nsprocessinfo/1414876-isoperatingsystematleastversion?language=objc

Add extra exception catcher to cope with OSX builds failing to catch
boost::locale::conv::conversion_error and the like with std:exception.

Brecht Van Lommel (brecht) added inline comments.
intern/locale/boost_locale_wrapper.cpp
122

catched -> caught

This revision is now accepted and ready to land.Oct 28 2021, 12:30 PM

const, check against nullable languageCode, typos, formatting

Found a bug report similar to this. https://bugs.llvm.org/show_bug.cgi?id=37487 Don't know if any of the dependencies are responsible in our case or if it's a different issue.
Still present in clang version 13.0.0 (https://github.com/llvm/llvm-project.git 3b47bd32f9df4a57db98db5f35e680c7bd9fde3e), tested that gist.

However, the conversion_error is being caught properly now. 2.93, compiler version above. But since it's still happening with all those Big Sur users (or with blender compiled on a specific system), let's keep that extra catcher.

diff --git a/intern/locale/boost_locale_wrapper.cpp b/intern/locale/boost_locale_wrapper.cpp
index 73433fe7c5e..a5ed7caf53d 100644
--- a/intern/locale/boost_locale_wrapper.cpp
+++ b/intern/locale/boost_locale_wrapper.cpp
@@ -87,6 +87,7 @@ void bl_locale_set(const char *locale)
   // gen.set_default_messages_domain(default_domain);
 
   try {
+    throw boost::locale::conv::conversion_error();
     if (locale && locale[0]) {
       _locale = gen(locale);
     }
@@ -118,6 +119,7 @@ void bl_locale_set(const char *locale)
 #undef LOCALE_INFO
   }
   catch (std::exception const &e) {
+    std::cout << "Type:    " << typeid(e).name() << "\n";
     std::cout << "bl_locale_set(" << locale << "): " << e.what() << " \n";
   }
 }

Type: N5boost6locale4conv16conversion_errorE

Bastien Montagne (mont29) marked an inline comment as done.

Final updates from reviews..

Rebase on proper 3.0 branch.

Fix previous mess, sorry for the noise.

Thanks for the review and improvements, will commit to 3.0 now.