Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/crash loop detection #80

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,5 @@ hs_err_pid*
obj/
.externalNativeBuild
**/.cxx

backtrace-library/src/main/jniLibs/**
25 changes: 25 additions & 0 deletions backtrace-library/src/main/cpp/backends/backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,29 @@ void Disable() {
"Disable not supported on this backend");
#endif
}

bool EnableCrashLoopDetectionBackend() {
konst-sauce marked this conversation as resolved.
Show resolved Hide resolved
#ifdef CRASHPAD_BACKEND
return EnableCrashLoopDetectionCrashpad();
#elif BREAKPAD_BACKEND
return false;
konst-sauce marked this conversation as resolved.
Show resolved Hide resolved
#endif
}

bool IsSafeModeRequiredBackend() {
#ifdef CRASHPAD_BACKEND
return IsSafeModeRequiredCrashpad();
#elif BREAKPAD_BACKEND
return false;
#endif
}

int ConsecutiveCrashesCountBackend() {
#ifdef CRASHPAD_BACKEND
return ConsecutiveCrashesCountCrashpad();
#elif BREAKPAD_BACKEND
return 0;
#endif
}

}
29 changes: 26 additions & 3 deletions backtrace-library/src/main/cpp/backends/crashpad-backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ extern std::atomic_bool disabled;
static crashpad::CrashpadClient *client;
static std::unique_ptr<crashpad::CrashReportDatabase> database;

static int consecutive_crashes_count = 0;

bool InitializeCrashpad(jstring url,
jstring database_path,
jstring handler_path,
Expand All @@ -19,6 +21,7 @@ bool InitializeCrashpad(jstring url,
jobjectArray attachmentPaths,
jboolean enableClientSideUnwinding,
jint unwindingMode) {

konst-sauce marked this conversation as resolved.
Show resolved Hide resolved
// avoid multi initialization
if (initialized) {
__android_log_print(ANDROID_LOG_ERROR, "Backtrace-Android", "Crashpad is already initialized");
Expand Down Expand Up @@ -120,9 +123,12 @@ bool InitializeCrashpad(jstring url,

// Start crash handler
client = new crashpad::CrashpadClient();
client->EnableCrashLoopDetection();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we want to make this configurable for the end-users.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should - imo we should pass it and don't execute this path if we don't need it. By default we want have features opt-in

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed it - it is called from Example's MainActivity now.


initialized = client->StartHandlerAtCrash(handler, db, db, backtraceUrl, attributes,
arguments);
// Get consecutive crashes count BEFORE any handler started,
// as it writes extra line into CSV, what leads to getting 0 for each next ConsecutiveCrashesCount call
consecutive_crashes_count = crashpad::CrashpadClient::ConsecutiveCrashesCount(db);
konst-sauce marked this conversation as resolved.
Show resolved Hide resolved
initialized = client->StartHandlerAtCrash(handler, db, db, backtraceUrl, attributes, arguments);

env->ReleaseStringUTFChars(url, backtraceUrl);
env->ReleaseStringUTFChars(handler_path, handlerPath);
Expand Down Expand Up @@ -219,10 +225,27 @@ void ReEnableCrashpad() {
// Re-enable uploads if disabled
if (disabled) {
if (database == nullptr) {
__android_log_print(ANDROID_LOG_ERROR, "Backtrace-Android", "Crashpad database is null, this should not happen");
__android_log_print(ANDROID_LOG_ERROR, "Backtrace-Android",
"Crashpad database is null, this should not happen");
return;
}
database->GetSettings()->SetUploadsEnabled(true);
disabled = false;
}
}

bool EnableCrashLoopDetectionCrashpad() {
konst-sauce marked this conversation as resolved.
Show resolved Hide resolved
if (client != nullptr) {
return client->EnableCrashLoopDetection();
} else {
return false;
}
}

bool IsSafeModeRequiredCrashpad() {
return consecutive_crashes_count >= 5;
}

int ConsecutiveCrashesCountCrashpad() {
return consecutive_crashes_count;
}
15 changes: 15 additions & 0 deletions backtrace-library/src/main/cpp/backtrace-native.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,19 @@ Java_backtraceio_library_BacktraceDatabase_disable(JNIEnv *env, jobject thiz) {
Disable();
}

JNIEXPORT jboolean JNICALL
Java_backtraceio_library_BacktraceDatabase_EnableCrashLoopDetection(JNIEnv *env, jclass clazz) {
return EnableCrashLoopDetectionBackend();
}

JNIEXPORT jboolean JNICALL
Java_backtraceio_library_BacktraceDatabase_IsSafeModeRequired(JNIEnv *env, jclass clazz) {
return IsSafeModeRequiredBackend();
}

JNIEXPORT int JNICALL
Java_backtraceio_library_BacktraceDatabase_ConsecutiveCrashesCount(JNIEnv *env, jclass clazz) {
return ConsecutiveCrashesCountBackend();
}

}
2 changes: 1 addition & 1 deletion backtrace-library/src/main/cpp/crashpad
Submodule crashpad updated 1124 files
6 changes: 6 additions & 0 deletions backtrace-library/src/main/cpp/include/backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ void DumpWithoutCrash(jstring message, jboolean set_main_thread_as_faulting_thre
void AddAttribute(jstring key, jstring value);

void Disable();

bool EnableCrashLoopDetectionBackend();

bool IsSafeModeRequiredBackend();

int ConsecutiveCrashesCountBackend();
}

#endif //BACKTRACE_ANDROID_BACKEND_H
4 changes: 4 additions & 0 deletions backtrace-library/src/main/cpp/include/crashpad-backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ void DisableCrashpad();

void ReEnableCrashpad();

bool EnableCrashLoopDetectionCrashpad();
bool IsSafeModeRequiredCrashpad();
int ConsecutiveCrashesCountCrashpad();

#endif //BACKTRACE_ANDROID_CRASHPAD_BACKEND_H
Original file line number Diff line number Diff line change
Expand Up @@ -464,4 +464,8 @@ private boolean validateDatabaseSize() {
public long getDatabaseSize() {
return backtraceDatabaseContext.getDatabaseSize();
}

public static native boolean EnableCrashLoopDetection();
public static native boolean IsSafeModeRequired();
public static native int ConsecutiveCrashesCount();
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package backtraceio.backtraceio;

import static backtraceio.backtraceio.BuildConfig.BACKTRACE_SUBMISSION_URL;

import android.content.Context;
import android.os.Bundle;
import androidx.appcompat.app.AppCompatActivity;
Expand Down Expand Up @@ -27,6 +29,7 @@
import backtraceio.library.enums.BacktraceBreadcrumbType;
import backtraceio.library.enums.database.RetryBehavior;
import backtraceio.library.enums.database.RetryOrder;
import backtraceio.library.models.BacktraceData;
import backtraceio.library.models.BacktraceExceptionHandler;
import backtraceio.library.models.BacktraceMetricsSettings;
import backtraceio.library.models.database.BacktraceDatabaseSettings;
Expand All @@ -49,10 +52,22 @@ protected void onCreate(Bundle savedInstanceState) {
setContentView(R.layout.activity_main);

// Set this value in your local.properties
if (BuildConfig.BACKTRACE_SUBMISSION_URL != null) {
backtraceClient = initializeBacktrace(BuildConfig.BACKTRACE_SUBMISSION_URL);
if (BACKTRACE_SUBMISSION_URL != null) {
backtraceClient = initializeBacktrace(BACKTRACE_SUBMISSION_URL);
}

// Crash Loop Detector example
boolean isCLSafeModeReq = BacktraceDatabase.IsSafeModeRequired();
konraddysput marked this conversation as resolved.
Show resolved Hide resolved
int crashesCount = BacktraceDatabase.ConsecutiveCrashesCount();
Log.i("BacktraceAndroid", "ConsecutiveCrashesCount: " + crashesCount);

View viewBackground = findViewById(R.id.viewBackground);
if(viewBackground != null) {
konst-sauce marked this conversation as resolved.
Show resolved Hide resolved
viewBackground.setBackgroundColor(isCLSafeModeReq
? getResources().getColor(R.color.colorAccent)
: getResources().getColor(R.color.colorWhite));
}

symlinkAndWriteFile();
}

Expand Down
8 changes: 7 additions & 1 deletion example-app/src/main/res/layout/activity_main.xml
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
<androidx.constraintlayout.widget.ConstraintLayout
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="match_parent"
tools:context=".MainActivity">

<View
android:id="@+id/viewBackground"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:background="@color/colorWhite"/>
<Button
android:id="@+id/handledException"
android:layout_width="wrap_content"
Expand Down
1 change: 1 addition & 0 deletions example-app/src/main/res/values/colors.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
<color name="colorPrimary">#008577</color>
<color name="colorPrimaryDark">#00574B</color>
<color name="colorAccent">#D81B60</color>
<color name="colorWhite">#FFFFFF</color>
</resources>