Skip to content

Commit

Permalink
Load library outside instance creation (#4595)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/488551667048375/1207420696563461/f

### Description
Load native crash library off class instance construction

### Steps to test this PR

_Test_
- [x] fresh install and/or update from this branch
- [x] open the app
- [x] verify `Native crash handler init pixel sent on main` shows in logcat
- [x] verify `Native crash handler successfully initialized on main` shows in logcat
- [x] enable AppTP
- [x] verify `Native crash handler init pixel sent on vpn` shows in logcat
- [x] verify `Native crash handler successfully initialized on vpn` shows in logcat
- [x] fire button
- [x] verify `Native crash handler init pixel sent on main` shows in logcat
- [x] verify `Native crash handler successfully initialized on main` shows in logcat
- [x] execute `adb shell am force-stop com.duckduckgo.mobile.android.debug`
- [x] launch app
- [x] verify `Native crash handler init pixel sent on main` shows in logcat
- [x] verify `Native crash handler successfully initialized on main` shows in logcat
- [x] verify `Native crash handler init pixel sent on vpn` shows in logcat
- [x] verify `Native crash handler successfully initialized on vpn` shows in logcat
- [x] disable AppTP
- [x] execute `adb shell am force-stop com.duckduckgo.mobile.android.debug` and re-launch app
- [x] verify `Native crash handler init pixel sent on main` shows in logcat
- [x] verify `Native crash handler successfully initialized on main` shows in logcat

_Test main process crash_
- [x] Apply the following patch, rebuild and fresh install
```diff
Subject: [PATCH] fix
---
Index: anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt b/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt
--- a/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt	(revision 3eb8033)
+++ b/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt	(date 1716898016639)
@@ -35,6 +35,7 @@
 import logcat.LogPriority.ERROR
 import logcat.asLog
 import logcat.logcat
+import kotlin.concurrent.thread
 
 @ContributesMultibinding(
     scope = AppScope::class,
@@ -57,10 +58,15 @@
     private val processName: String by lazy { if (isMainProcess) "main" else "vpn" }
 
     private external fun jni_register_sighandler(logLevel: Int, appVersion: String, processName: String, isCustomTab: Boolean)
+    private external fun jni_crash()
 
     override fun onCreate(owner: LifecycleOwner) {
         if (isMainProcess) {
             asyncLoadNativeLibrary()
+            thread {
+                Thread.sleep(2000)
+                jni_crash()
+            }
         } else {
             logcat(ERROR) { "ndk-crash: onCreate wrongly called in a secondary process" }
         }
Index: anrs/anrs-impl/src/main/cpp/jni.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/anrs/anrs-impl/src/main/cpp/jni.cpp b/anrs/anrs-impl/src/main/cpp/jni.cpp
--- a/anrs/anrs-impl/src/main/cpp/jni.cpp	(revision 3eb8033)
+++ b/anrs/anrs-impl/src/main/cpp/jni.cpp	(date 1716897983303)
@@ -74,3 +74,19 @@
         jobject /* this */) {
     native_crash_handler_fini();
 }
+
+// Our custom test exception. Anything "publicly" inheriting std::exception will work
+class MyException : public std::exception {
+public:
+    const char* what() const noexcept override {
+        return "This is a really important crash message!";
+    }
+};
+
+extern "C" JNIEXPORT void JNICALL
+Java_com_duckduckgo_app_anr_ndk_NativeCrashInit_jni_1crash(
+        JNIEnv* env,
+        jobject /* this */) {
+    throw MyException(); // This can be replaced with any foreign function call that throws.
+}
+
```
- [x] launch app
- [x] verify `Native crash handler init pixel sent on main` shows in logcat
- [x] verify `Native crash handler successfully initialized on main` shows in logcat
- [x] verify `Native crash pixel sent on main` shows in logcat
- [x] verify app crashed too

_Test crash in vpn process_
- [x] Apply the following patch, rebuild and fresh install
```diff
Subject: [PATCH] fix
---
Index: anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt b/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt
--- a/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt	(revision 3eb8033)
+++ b/anrs/anrs-impl/src/main/java/com/duckduckgo/app/anr/ndk/NativeCrashInit.kt	(date 1716990529537)
@@ -35,6 +35,7 @@
 import logcat.LogPriority.ERROR
 import logcat.asLog
 import logcat.logcat
+import kotlin.concurrent.thread
 
 @ContributesMultibinding(
     scope = AppScope::class,
@@ -57,6 +58,7 @@
     private val processName: String by lazy { if (isMainProcess) "main" else "vpn" }
 
     private external fun jni_register_sighandler(logLevel: Int, appVersion: String, processName: String, isCustomTab: Boolean)
+    private external fun jni_crash()
 
     override fun onCreate(owner: LifecycleOwner) {
         if (isMainProcess) {
@@ -69,6 +71,10 @@
     override fun onVpnProcessCreated() {
         if (!isMainProcess) {
             asyncLoadNativeLibrary()
+            thread {
+                Thread.sleep(2000)
+                jni_crash()
+            }
         } else {
             logcat(ERROR) { "ndk-crash: onCreate wrongly called in the main process" }
         }
Index: anrs/anrs-impl/src/main/cpp/jni.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/anrs/anrs-impl/src/main/cpp/jni.cpp b/anrs/anrs-impl/src/main/cpp/jni.cpp
--- a/anrs/anrs-impl/src/main/cpp/jni.cpp	(revision 3eb8033)
+++ b/anrs/anrs-impl/src/main/cpp/jni.cpp	(date 1716990529535)
@@ -74,3 +74,19 @@
         jobject /* this */) {
     native_crash_handler_fini();
 }
+
+// Our custom test exception. Anything "publicly" inheriting std::exception will work
+class MyException : public std::exception {
+public:
+    const char* what() const noexcept override {
+        return "This is a really important crash message!";
+    }
+};
+
+extern "C" JNIEXPORT void JNICALL
+Java_com_duckduckgo_app_anr_ndk_NativeCrashInit_jni_1crash(
+        JNIEnv* env,
+        jobject /* this */) {
+    throw MyException(); // This can be replaced with any foreign function call that throws.
+}
+
```
- [x] launch app and enable AppTP
- [x] verify `Native crash handler init pixel sent on vpn` shows in logcat
- [x] verify `Native crash handler successfully initialized on vpn` shows in logcat
- [x] verify `Native crash pixel sent on vpn` shows in logcat
(The VPN will automatically try to re-start, creating and endless loop, expected)
  • Loading branch information
aitorvs authored May 29, 2024
1 parent a12b7a6 commit 313b651
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,18 @@ import android.content.Context
import android.util.Log
import androidx.lifecycle.LifecycleOwner
import com.duckduckgo.app.browser.customtabs.CustomTabDetector
import com.duckduckgo.app.di.AppCoroutineScope
import com.duckduckgo.app.di.IsMainProcess
import com.duckduckgo.app.lifecycle.MainProcessLifecycleObserver
import com.duckduckgo.app.lifecycle.VpnProcessLifecycleObserver
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.appbuildconfig.api.isInternalBuild
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.checkMainThread
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.library.loader.LibraryLoader
import com.duckduckgo.library.loader.LibraryLoader.LibraryLoaderListener
import com.squareup.anvil.annotations.ContributesMultibinding
import dagger.SingleInstanceIn
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import logcat.LogPriority.ERROR
import logcat.asLog
import logcat.logcat
Expand All @@ -49,52 +46,43 @@ import logcat.logcat
)
@SingleInstanceIn(AppScope::class)
class NativeCrashInit @Inject constructor(
context: Context,
private val context: Context,
@IsMainProcess private val isMainProcess: Boolean,
private val customTabDetector: CustomTabDetector,
private val appBuildConfig: AppBuildConfig,
private val nativeCrashFeature: NativeCrashFeature,
private val dispatcherProvider: DispatcherProvider,
@AppCoroutineScope private val coroutineScope: CoroutineScope,
) : MainProcessLifecycleObserver, VpnProcessLifecycleObserver {
) : MainProcessLifecycleObserver, VpnProcessLifecycleObserver, LibraryLoaderListener {

private val isCustomTab: Boolean by lazy { customTabDetector.isCustomTab() }
private val processName: String by lazy { if (isMainProcess) "main" else "vpn" }

init {
try {
LibraryLoader.loadLibrary(context, "crash-ndk")
} catch (ignored: Throwable) {
logcat(ERROR) { "ndk-crash: Error loading crash-ndk lib: ${ignored.asLog()}" }
}
}

private external fun jni_register_sighandler(logLevel: Int, appVersion: String, processName: String, isCustomTab: Boolean)

override fun onCreate(owner: LifecycleOwner) {
if (isMainProcess) {
coroutineScope.launch {
jniRegisterNativeSignalHandler()
}
asyncLoadNativeLibrary()
} else {
logcat(ERROR) { "ndk-crash: onCreate wrongly called in a secondary process" }
}
}

override fun onVpnProcessCreated() {
if (!isMainProcess) {
coroutineScope.launch {
jniRegisterNativeSignalHandler()
}
asyncLoadNativeLibrary()
} else {
logcat(ERROR) { "ndk-crash: onCreate wrongly called in the main process" }
}
}

private suspend fun jniRegisterNativeSignalHandler() = withContext(dispatcherProvider.io()) {
override fun success() {
// do not call on main thread
checkMainThread()

runCatching {
if (isMainProcess && !nativeCrashFeature.nativeCrashHandling().isEnabled()) return@withContext
if (!isMainProcess && !nativeCrashFeature.nativeCrashHandlingSecondaryProcess().isEnabled()) return@withContext
logcat(ERROR) { "ndk-crash: Library loaded in process $processName" }

if (isMainProcess && !nativeCrashFeature.nativeCrashHandling().isEnabled()) return
if (!isMainProcess && !nativeCrashFeature.nativeCrashHandlingSecondaryProcess().isEnabled()) return

val logLevel = if (appBuildConfig.isDebug || appBuildConfig.isInternalBuild()) {
Log.VERBOSE
Expand All @@ -106,4 +94,12 @@ class NativeCrashInit @Inject constructor(
logcat(ERROR) { "ndk-crash: Error calling jni_register_sighandler: ${it.asLog()}" }
}
}

override fun failure(t: Throwable?) {
logcat(ERROR) { "ndk-crash: error loading library in process $processName: ${t?.asLog()}" }
}

private fun asyncLoadNativeLibrary() {
LibraryLoader.loadLibrary(context, "crash-ndk", this)
}
}
2 changes: 1 addition & 1 deletion library-loader/library-loader-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ apply from: "$rootProject.projectDir/gradle/android-library.gradle"

dependencies {

implementation "com.getkeepsafe.relinker:relinker:_"
api "com.getkeepsafe.relinker:relinker:_"
}

android {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,18 @@ package com.duckduckgo.library.loader

import android.content.Context
import com.getkeepsafe.relinker.ReLinker
import com.getkeepsafe.relinker.ReLinker.LoadListener

class LibraryLoader {
companion object {
fun loadLibrary(context: Context, name: String) {
ReLinker.loadLibrary(context, name)
}

fun loadLibrary(context: Context, name: String, listener: LibraryLoaderListener) {
ReLinker.loadLibrary(context, name, listener)
}
}

interface LibraryLoaderListener : LoadListener
}

0 comments on commit 313b651

Please sign in to comment.