getsentry / sentry-unity

Development of Sentry SDK for Unity
https://docs.sentry.io/platforms/unity/
MIT License
207 stars 51 forks source link

il2cpp::gc::GCHandle::GetTarget #1442

Closed oddgames-david closed 2 months ago

oddgames-david commented 1 year ago

Using Sentry 1.5.2 on Android IL2CPP build Unity 2023.1.12f1 I'm getting a few of these reports.

https://odd-games.sentry.io/share/issue/0696247855b64ca9b9b89104b37e9dd5/

libil2cpp
+0x36985d4
il2cpp::gc::GCHandle::GetTarget (GCHandle.cpp:267)

Registers
fp
0x0000007d00f03a20
lr
0x0000007cea7f029c
pc
0x0000007ce6ac15d4
sp
0x0000007d00f03560
x0
0x0000000050248028
x1
0x0000007ca30d8b40
x10
0x0000000000000100
x11
0x0000000000000000
x12
0x0000000000000001
x13
0x000000007ff83ffe
x14
0x000000000000692d
x15
0x0000000000000500
x16
0x0000007cebb1cdf8
x17
0x0000007ce6abdf1c
x18
0x0000007ca1d8bc08
x19
0x0000007e27ea2ba0
x2
0x0000007ca30d8b40
x20
0x0000007b3364abe0
x21
0x0000007cebfb7000
x22
0x0000000050248000
x23
0x0000007cebc65258
x24
0x0000007cebc5da90
x25
0x0000007b34ceb910
x26
0x0000007b335cff00
x27
0x0000007b34ceb370
x28
0x0000007e27ea2ba0
x3
0x0000007cea1fea24
x4
0x0000007d00f03528
x5
0x0000000000000000
x6
0x0000000000000000
x7
0x0000007d00f033c0
x8
0x0000007d9f608120
x9
0x0000007ce6a5b114

Show More
libil2cpp
+0x73c7298
Il2CppGcHandleGetTarget_Invoke_m6FEE8370F30250FA5751017CC620CE73AE0CB35B_inline (Sentry.Unity.cpp:28416)
libil2cpp
+0x73c7298
UnityIl2CppEventExceptionProcessor_GetNativeStackTrace_mBB0ED1998D356AFACF7660200CC22DAC216112E2 (Sentry.Unity.cpp:7676)
libil2cpp
+0x73c644c
UnityIl2CppEventExceptionProcessor_Process_m5FA4F24DA0C01E2EB446EDB1292FD66FB188C407 (Sentry.Unity.cpp:6654)
libil2cpp
+0x72a46ac
InterfaceActionInvoker2<T>::Invoke (Sentry.cpp:124)
libil2cpp
+0x72a46ac
SentryClient_DoSendEvent_m9741507AB2309CC5A5D884756AF5F1F31202E44C (Sentry.cpp:33715)
libil2cpp
+0x72a3ee0
SentryClient_CaptureEvent_mC0AD74C6F6C740860341C0E06A976FFEC71832E0 (Sentry.cpp:32931)
libil2cpp
+0x741e75c
InterfaceFuncInvoker3<T>::Invoke (Sentry__3.cpp:156)
libil2cpp
+0x741e75c
Hub_Sentry_Internal_IHubEx_CaptureEventInternal_mA56BE6410AA18C60B03AB60399855C2FDD0C42BC (Sentry__3.cpp:20470)
libil2cpp
+0x73dc4d4
SentryClientExtensions_CaptureException_m86DE8108C63794599A74AF2D508F1B932A6A7835 (Sentry__1.cpp:5955)
libil2cpp
+0x73db9fc
UnityLogHandlerIntegration_CaptureException_mD5B88C4A6E3FCF9C87C1D9BD062A327E1E7E1DE3 (Sentry.Unity.cpp:26979)
libil2cpp
+0x73db600
UnityLogHandlerIntegration_LogException_m23CB511805A3C839541A9868BC1FD148CF9A9E47 (Sentry.Unity.cpp:26861)
libil2cpp
+0x3a50490
InterfaceActionInvoker0::Invoke (Assembly-CSharp__33.cpp:79)
libil2cpp
+0x3a50490
ParseClient_Initialize_mF9591AA6524B5F14A0D84F473BD6CF028CE94334 (ParseClient.cs:124)
libil2cpp
+0x39c98cc
ParseManager_Initialize_m06FE5EC4111F4447FDD8068E2B0E2FF430D6FE3E (ParseManager.cs:51)
libil2cpp
+0x3c3c198
U3COpenU3Ed__6_MoveNext_m23FC6436C91B6B5E39E9027121FD0E34285DA9C4 (GameModeLevelEditor.cs:45)
libil2cpp
+0x7dd1c94
InterfaceFuncInvoker0<T>::Invoke (UnityEngine.CoreModule__3.cpp:123)
libil2cpp
+0x7dd1c94
SetupCoroutine_InvokeMoveNext_m72FC77384CAC3133B6EE650E0581D055B34B2F5F (UnityEngine.CoreModule__3.cpp:8147)
libil2cpp
+0x36f3428
il2cpp::vm::Runtime::InvokeWithThrow (Runtime.cpp:639)
libil2cpp
+0x36f3374
il2cpp::vm::Runtime::Invoke (Runtime.cpp:625)
bruno-garcia commented 1 year ago

Thanks for raising. Took a look and doing a dump here to help folks (and also because shared event expired in 90 days):

11 different events, all Android 13 but 3 different device models (Samsung, OnePlus, etc) Symbols seemed to have been uploaded (other than the Android system symbol).

Looks like a LogException was called in Unity: UnityLogHandlerIntegration_LogException_m23CB511805A3C839541A9868BC1FD148CF9A9E47 which called UnityLogHandlerIntegration_CaptureException_mD5B88C4A6E3FCF9C87C1D9BD062A327E1E7E1DE3 and through out integration got into the Sentry SDK.

Then goes into:

https://github.com/getsentry/sentry-unity/blob/ea2fe9c3ddef8295af9432a7038bafcbcbf4c03d/src/Sentry.Unity/Il2CppEventProcessor.cs#L315

Somehow in there, two frames in, it crashes.

Il2CppGcHandleGetTarget_Invoke_m6FEE8370F30250FA5751017CC620CE73AE0CB35B_inline il2cpp::gc::GCHandle::GetTarget (GCHandle.cpp:267)

Finally seems to crash here:

https://github.com/getsentry/sentry-unity/blob/ea2fe9c3ddef8295af9432a7038bafcbcbf4c03d/src/Sentry.Unity/Il2CppEventProcessor.cs#L326

Error captured from from Sentry Native 0.6.5 which got into the Android SDK on version 6.25.1. That got into Unity on 1.5.1

@Swatinem should we be looking at C# instead of C++ given all symbols were uploaded?

Swatinem commented 1 year ago

The problem is rather that we are crashing trying to resolve the C# provided stack trace. In particular, converting the C# object into its underlying C++ object/pointer so that we can call unity internal C++ methods on it. Its possible that a new version of il2cpp works slightly differently and is now incompatible with out hacks.

oddgames-david commented 1 year ago

Something to do with line numbers now apparently being available? image

Swatinem commented 1 year ago

That might be possible. I haven’t looked at this in detail in quite a while.

As a workaround, if this is causing problems for you, you can disable Sentrys own il2cpp line numbers integration, that should avoid running the code that is causing the crash here.

bitsandfoxes commented 1 year ago

With Unity 2023 the signature for il2cpp_gchandle_get_target changed from

Il2CppObject* il2cpp_gchandle_get_target(uint32_t gchandle)

to

Il2CppObject* il2cpp_gchandle_get_target(Il2CppGCHandle gchandle)

With typedef void* Il2CppGCHandle; So we "just" need to get that change reflected somewhere in the #if in SentryInitialization.

I managed to get it working locally.

dganzella commented 3 months ago

@bitsandfoxes my baby boy, I think this issue might have come back with unity 6. Its crashing exactly the same way, exactly on the same line, and disabling the line thingy makes it not crash anymore

image

dganzella commented 3 months ago

Crashes caused by sentry are extremely frustrating because at a glance you first think that sentry its just trying to report a crash caused by another circumstance, not crashing the app itself

having sentry actually crashing the app is such a curveball, I really would suggest this feature to be disabled by default and marked as experimental @bitsandfoxes

bitsandfoxes commented 3 months ago

Damn.. thanks @dganzella for pointing that out. We're going to have to update our CI to include Unity 6 at the very least!

bitsandfoxes commented 2 months ago

Working on it! This has not been forgotten.