Fix deadlock if ensure event captured during garbage collection (#860)

* Fix deadlock if ensure captured durig garbage collection

* Add GC check for mobile

* Update README.md

Co-authored-by: Stefan Jandl <[email protected]>

---------

Co-authored-by: Stefan Jandl <[email protected]>

Author: tustanivsky

CHANGELOG.md

@@ -14,9 +14,6 @@
 ### Fixes
 
 - Fix crash during garbage collection if SentryId was instantiated outside of game thread ([#857](https://github.com/getsentry/sentry-unreal/pull/857))
-
-### Fixes
-
 - Fix ensure when log message from non-game thread ([#845](https://github.com/getsentry/sentry-unreal/pull/845))
 
 ### Dependencies

README.md

@@ -53,7 +53,7 @@ Blog posts:
 
 - Only crash events captured on Android contain the full callstack. Events that were captured manually won't have the native C++ part there.
 
-- On Windows/Linux if crash event was captured during the garbage collection the `BeforeSendHandler` will not be invoked.
+- If an event was captured during the garbage collection, the `BeforeSendHandler` will not be invoked.
 
 - It may be required to upgrade the C++ standard library (`libstdc++`) on older Linux distributions (such as Ubuntu 18.04 and 20.04) to ensure crashpad handler proper functionality within the deployment environment. This can be achieved with something like this:
 ```

plugin-dev/Source/Sentry/Private/Android/Jni/SentryJniAndroid.cpp

@@ -41,11 +41,18 @@ JNI_METHOD void Java_io_sentry_unreal_SentryBridgeJava_onConfigureScope(JNIEnv*
 
 JNI_METHOD jobject Java_io_sentry_unreal_SentryBridgeJava_onBeforeSend(JNIEnv* env, jclass clazz, jlong objAddr, jobject event, jobject hint)
 {
-	FGCScopeGuard GCScopeGuard;
-
 	if (FUObjectThreadContext::Get().IsRoutingPostLoad)
 	{
-		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed when post-loading."));
+		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed during object post-loading."));
+		return event;
+	}
+
+	if (IsGarbageCollecting())
+	{
+		// If event is captured during garbage collection we can't instantiate UObjects safely or obtain a GC lock
+		// since it will cause a deadlock (see https://github.com/getsentry/sentry-unreal/issues/850).
+		// In this case event will be reported without calling a `beforeSend` handler.
+		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed during garbage collection."));
 		return event;
 	}
 

plugin-dev/Source/Sentry/Private/Apple/AppleSentrySubsystem.cpp

@@ -67,14 +67,21 @@ void FAppleSentrySubsystem::InitWithSettings(const USentrySettings* settings, US
 				return scope;
 			};
 			options.beforeSend = ^SentryEvent* (SentryEvent* event) {
-				FGCScopeGuard GCScopeGuard;
-
 				if (FUObjectThreadContext::Get().IsRoutingPostLoad)
 				{
 					UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed when post-loading."));
 					return event;
 				}
 
+				if (IsGarbageCollecting())
+				{
+					// If event is captured during garbage collection we can't instantiate UObjects safely or obtain a GC lock
+					// since it will cause a deadlock (see https://github.com/getsentry/sentry-unreal/issues/850).
+					// In this case event will be reported without calling a `beforeSend` handler.
+					UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed during garbage collection."));
+					return event;
+				}
+
 				USentryEvent* EventToProcess = USentryEvent::Create(MakeShareable(new SentryEventApple(event)));
 
 				USentryEvent* ProcessedEvent = beforeSendHandler->HandleBeforeSend(EventToProcess, nullptr);

plugin-dev/Source/Sentry/Private/GenericPlatform/GenericPlatformSentrySubsystem.cpp

@@ -92,16 +92,23 @@ sentry_value_t FGenericPlatformSentrySubsystem::OnBeforeSend(sentry_value_t even
 
 	GetCurrentScope()->Apply(Event);
 
-	FGCScopeGuard GCScopeGuard;
-
 	if (FUObjectThreadContext::Get().IsRoutingPostLoad)
 	{
-		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed when post-loading."));
+		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed during object post-loading."));
+		return event;
+	}
+
+	if (IsGarbageCollecting())
+	{
+		// If event is captured during garbage collection we can't instantiate UObjects safely or obtain a GC lock
+		// since it will cause a deadlock (see https://github.com/getsentry/sentry-unreal/issues/850).
+		// In this case event will be reported without calling a `beforeSend` handler.
+		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed during garbage collection."));
 		return event;
 	}
 
 	USentryEvent* EventToProcess = USentryEvent::Create(Event);
-	
+
 	USentryEvent* ProcessedEvent = GetBeforeSendHandler()->HandleBeforeSend(EventToProcess, nullptr);
 
 	return ProcessedEvent ? event : sentry_value_new_null();
@@ -127,26 +134,27 @@ sentry_value_t FGenericPlatformSentrySubsystem::OnCrash(const sentry_ucontext_t*
 
 	GetCurrentScope()->Apply(Event);
 
-	if (!IsGarbageCollecting())
+	if (FUObjectThreadContext::Get().IsRoutingPostLoad)
 	{
-		USentryEvent* EventToProcess = USentryEvent::Create(Event);
-
-		USentryEvent* ProcessedEvent = EventToProcess;
-		if (!FUObjectThreadContext::Get().IsRoutingPostLoad)
-		{
-			// Executing UFUNCTION is allowed only when not post-loading
-			ProcessedEvent = GetBeforeSendHandler()->HandleBeforeSend(EventToProcess, nullptr);
-		}
-
-		return ProcessedEvent ? event : sentry_value_new_null();
+		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed when post-loading."));
+		return event;
 	}
-	else
+
+	if (IsGarbageCollecting())
 	{
-		// If crash occurred during garbage collection we can't just obtain a GC lock like with normal events
-		// since there is no guarantee it will be ever freed. In this case crash event will be reported
-		// without calling a `beforeSend` handler.
+		// If crash occurred during garbage collection we can't instantiate UObjects safely or obtain a GC lock
+		// since there is no guarantee it will be ever freed.
+		// In this case crash event will be reported without calling a `beforeSend` handler.
+		UE_LOG(LogSentrySdk, Log, TEXT("Executing `beforeSend` handler is not allowed during garbage collection."));
 		return event;
 	}
+
+	USentryEvent* EventToProcess = USentryEvent::Create(Event);
+
+	USentryEvent* ProcessedEvent = GetBeforeSendHandler()->HandleBeforeSend(EventToProcess, nullptr);
+
+	return ProcessedEvent ? event : sentry_value_new_null();
+
 }
 
 FGenericPlatformSentrySubsystem::FGenericPlatformSentrySubsystem()