Convert iOS crash to lua_error

[Filazapovich]

After throw NSInvalidArgumentException from NSMutableDictionary setObject:forKey: the app is terminated. This crash is occurred on ~30% of iOS devices but i don't know how to reproduce it.

Exception Type:  EXC_CRASH (SIGABRT)
Exception Codes: 0x0000000000000000, 0x0000000000000000
Exception Note:  EXC_CORPSE_NOTIFY
Triggered by Thread:  0

Last Exception Backtrace:
0   CoreFoundation                  0x18e2a7180 __exceptionPreprocess + 228 (NSException.m:172)
1   libobjc.A.dylib                 0x18d47f9f8 objc_exception_throw + 56 (objc-exception.mm:557)
2   CoreFoundation                  0x18e220bec _CFThrowFormattedException + 112 (CFObject.m:1958)
3   CoreFoundation                  0x18e195a08 -[__NSDictionaryM setObject:forKey:] + 940 (NSDictionaryM.m:176)
4   FamilyIsland                    0x100f4204c firebase::analytics::LogEvent+ 3153996 (char const*, firebase::analytics::Parameter const*, unsigned long) + 548
5   FamilyIsland                    0x100c57564 Firebase_Analytics_LogTable(lua_State*) + 95588 (firebase.cpp:182)
6   FamilyIsland                    0x10108f8ec lj_BC_JFUNCV + 43
7   FamilyIsland                    0x1010ad01c lua_call + 75
8   FamilyIsland                    0x100c5d3c8 +[Utils execute_tasks:] + 119752 (utils.mm:155)
9   FamilyIsland                    0x100c61054 Extension_adjust_Update(dmExtension::Params*) + 135252 (extension.cpp:40)
10  FamilyIsland                    0x101078d94 dmScript::InternalUpdateExtensions(dmScript::Context*) + 4427156 (script_extensions.cpp:102)
11  FamilyIsland                    0x1010720a4 dmScript::Update(dmScript::Context*) + 4399268 (script.cpp:222)
12  FamilyIsland                    0x100c46d08 dmEngine::Step(dmEngine::Engine*) + 27912 (engine.cpp:0)
13  FamilyIsland                    0x10105a4fc dmGraphics::RunApplicationLoop(void*, void (*)(void*), int (*)(void*)) + 4302076 (graphics_opengl.cpp:765)
14  FamilyIsland                    0x100c47b64 dmEngine::InitRun(dmEngineService::EngineService*, int, char**, void (*)(dmEngine::Engine*, void*), void (*)(dmEngine::Engine*, void*), void*) + 31588 (engine.cpp:1464)
15  FamilyIsland                    0x100c479f0 dmEngine::Launch(int, char**, void (*)(dmEngine::Engine*, void*), void (*)(dmEngine::Engine*, void*), void*) + 31216 (engine.cpp:1492)
16  FamilyIsland                    0x100c47c44 engine_main(int, char**) + 31812 (engine_main.cpp:35)
17  libdyld.dylib                   0x18dcf68e0 start + 4

Thread 0 name:
Thread 0 Crashed:
0   libsystem_kernel.dylib          0x000000018de430dc __pthread_kill + 8
1   libsystem_pthread.dylib         0x000000018debc094 pthread_kill$VARIANT$mp + 380 (pthread.c:1492)
2   libsystem_c.dylib               0x000000018dd9bf4c __abort + 152 (abort.c:131)
3   libsystem_c.dylib               0x000000018dd9beb4 abort + 152 (abort.c:102)
4   libc++abi.dylib                 0x000000018d468788 abort_message + 132 (abort_message.cpp:75)
5   libc++abi.dylib                 0x000000018d468934 default_terminate_handler() + 308 (cxa_default_handlers.cpp:68)
6   libobjc.A.dylib                 0x000000018d47fe00 _objc_terminate() + 124 (objc-exception.mm:693)
7   libc++abi.dylib                 0x000000018d474838 std::__terminate(void (*)()) + 16 (cxa_handlers.cpp:66)
8   libc++abi.dylib                 0x000000018d4741a8 __cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 32 (cxa_exception.cpp:130)
9   libc++abi.dylib                 0x000000018d474168 __cxa_throw + 124 (cxa_exception.cpp:256)
10  libobjc.A.dylib                 0x000000018d47fb3c objc_exception_throw + 380 (objc-exception.mm:583)
11  CoreFoundation                  0x000000018e220bec _CFThrowFormattedException + 112 (CFObject.m:1958)
12  CoreFoundation                  0x000000018e195a08 -[__NSDictionaryM setObject:forKey:] + 940 (NSDictionaryM.m:176)
13  FamilyIsland                    0x0000000100f4204c firebase::analytics::LogEvent+ 3153996 (char const*, firebase::analytics::Parameter const*, unsigned long) + 548
14  FamilyIsland                    0x0000000100c57564 Firebase_Analytics_LogTable(lua_State*) + 95588 (firebase.cpp:184)
15  FamilyIsland                    0x000000010108f8ec lj_BC_JFUNCV + 44
16  FamilyIsland                    0x00000001010ad01c lua_call + 76
17  FamilyIsland                    0x0000000100c5d3c8 +[Utils execute_tasks:] + 119752 (utils.mm:156)
18  FamilyIsland                    0x0000000100c61054 Extension_adjust_Update(dmExtension::Params*) + 135252 (extension.cpp:41)
19  FamilyIsland                    0x0000000101078d94 dmScript::InternalUpdateExtensions(dmScript::Context*) + 4427156 (script_extensions.cpp:103)
20  FamilyIsland                    0x00000001010720a4 dmScript::Update(dmScript::Context*) + 4399268 (script.cpp:218)
21  FamilyIsland                    0x0000000100c46d08 dmEngine::Step(dmEngine::Engine*) + 27912 (engine.cpp:1261)
22  FamilyIsland                    0x000000010105a4fc dmGraphics::RunApplicationLoop(void*, void (*)(void*), int (*)(void*)) + 4302076 (graphics_opengl.cpp:763)
23  FamilyIsland                    0x0000000100c47b64 dmEngine::InitRun(dmEngineService::EngineService*, int, char**, void (*)(dmEngine::Engine*, void*), void (*)(dmEngine::Engine*, void*), void*) + 31588 (engine.cpp:1465)
24  FamilyIsland                    0x0000000100c479f0 dmEngine::Launch(int, char**, void (*)(dmEngine::Engine*, void*), void (*)(dmEngine::Engine*, void*), void*) + 31216 (engine.cpp:1493)
25  FamilyIsland                    0x0000000100c47c44 engine_main(int, char**) + 31812 (engine_main.cpp:35)
26  libdyld.dylib                   0x000000018dcf68e0 start + 4

[britzl]

It is great that we catch the crash and log but I would rather prefer that we find the cause as well. The crash seems to come from the Firebase_Analytics_LogTable function. Could it be that a string is too long?

https://github.com/defold/extension-firebase-analytics/blob/master/firebase/src/firebase.cpp#L161

What do you typically log?

Could it be that you log something other than a string, boolean or number?

https://github.com/defold/extension-firebase-analytics/blob/master/firebase/src/firebase.cpp#L169-L175

In this case I've noticed that size is still increased:

https://github.com/defold/extension-firebase-analytics/blob/master/firebase/src/firebase.cpp#L179

EDIT: Ah, nvm, if we try to log a value of the wrong type we bail the whole thing. So the question remains, are you logging any large strings?

[Filazapovich]

Nope. This crash is caused by NSInvalidArgumentException from NSMutableDictionary setObject:forKey:. I'd try to reproduce it with every lua type. And try to reproduce with nil or NULL values and param name. Look at firebase C++ src: https://github.com/firebase/firebase-cpp-sdk/blob/master/analytics/src/analytics_ios.mm#L192 and https://github.com/firebase/firebase-cpp-sdk/blob/master/analytics/src/analytics_ios.mm#L154 . Every invalid parameter value is handled here. And i'd try NULL name - nothing.

I don't see in source code anything about large strings. size variable is about count of parameters.

[britzl]

Remember to check v 5.7.0 of the code, not latest master! (although the look the same from a cursory glance)

https://github.com/firebase/firebase-cpp-sdk/blob/v5.7.0/analytics/src/analytics_ios.mm

[britzl]

And you are sure that the key isn't nil? lua_tostring() returns char* or NULL.

We do not check this in the extension and I believe analytics::Parameter also accepts a null value.

https://github.com/defold/extension-firebase-analytics/blob/master/firebase/src/firebase.cpp#L157

[Filazapovich]

Nil key can't exit in lua i think. I'd try to create Parameter with NULL key - handled by https://github.com/firebase/firebase-cpp-sdk/blob/v5.7.0/analytics/src/analytics_ios.mm#L68

[britzl]

Correct a Lua table can't have nil keys, but keys can be of any type and lua_tostring() will return NULL if the type is not string or number:

"The Lua value must be a string or a number; otherwise, the function returns NULL"

But on the other hand it seems like a null name/key is handled in the Firebase code... really weird.

[britzl]

May I suggest that we add some logging in your PR (and not only the lua_error). Could we maybe iterate over the parameters and use dmLogInfo() to print each key-value pair?

[britzl]

Thanks. Let's try this version then and if you are able to gather some data to pinpoint the problem please share it so that we can fix this issue properly!