Time limitation for thread collection (#1935)

* fix(threadStack timeout) add time limitation for thread stack

* fix(threadStack timeout) add time limitation for thread stack

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## TBC
+
+### Enhancements
+
+* The maximum time taken to collect Event.threads can now be controlled using 'Configuration.threadCollectionTimeLimitMillis' (default 5000ms)
+  [#1935](https://github.com/bugsnag/bugsnag-android/pull/1935)
+
 ## 6.0.0 (2023-11-20)
 
 ### Breaking Changes

bugsnag-android-core/api/bugsnag-android-core.api

@@ -192,6 +192,7 @@ public class com/bugsnag/android/Configuration : com/bugsnag/android/CallbackAwa
 	public fun getSendLaunchCrashesSynchronously ()Z
 	public fun getSendThreads ()Lcom/bugsnag/android/ThreadSendPolicy;
 	public fun getTelemetry ()Ljava/util/Set;
+	public fun getThreadCollectionTimeLimitMillis ()J
 	public fun getUser ()Lcom/bugsnag/android/User;
 	public fun getVersionCode ()Ljava/lang/Integer;
 	public fun isAttemptDeliveryOnCrash ()Z
@@ -228,6 +229,7 @@ public class com/bugsnag/android/Configuration : com/bugsnag/android/CallbackAwa
 	public fun setSendLaunchCrashesSynchronously (Z)V
 	public fun setSendThreads (Lcom/bugsnag/android/ThreadSendPolicy;)V
 	public fun setTelemetry (Ljava/util/Set;)V
+	public fun setThreadCollectionTimeLimitMillis (J)V
 	public fun setUser (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
 	public fun setVersionCode (Ljava/lang/Integer;)V
 }
@@ -859,7 +861,7 @@ public final class com/bugsnag/android/internal/DateUtils {
 }
 
 public final class com/bugsnag/android/internal/ImmutableConfig {
-	public fun <init> (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIILkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)V
+	public fun <init> (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)V
 	public final fun component1 ()Ljava/lang/String;
 	public final fun component10 ()Ljava/util/Set;
 	public final fun component11 ()Ljava/lang/String;
@@ -877,21 +879,22 @@ public final class com/bugsnag/android/internal/ImmutableConfig {
 	public final fun component22 ()I
 	public final fun component23 ()I
 	public final fun component24 ()I
-	public final fun component25 ()Lkotlin/Lazy;
-	public final fun component26 ()Z
+	public final fun component25 ()J
+	public final fun component26 ()Lkotlin/Lazy;
 	public final fun component27 ()Z
-	public final fun component28 ()Landroid/content/pm/PackageInfo;
-	public final fun component29 ()Landroid/content/pm/ApplicationInfo;
+	public final fun component28 ()Z
+	public final fun component29 ()Landroid/content/pm/PackageInfo;
 	public final fun component3 ()Lcom/bugsnag/android/ErrorTypes;
-	public final fun component30 ()Ljava/util/Collection;
+	public final fun component30 ()Landroid/content/pm/ApplicationInfo;
+	public final fun component31 ()Ljava/util/Collection;
 	public final fun component4 ()Z
 	public final fun component5 ()Lcom/bugsnag/android/ThreadSendPolicy;
 	public final fun component6 ()Ljava/util/Collection;
 	public final fun component7 ()Ljava/util/Collection;
 	public final fun component8 ()Ljava/util/Collection;
 	public final fun component9 ()Ljava/util/Set;
-	public final fun copy (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIILkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)Lcom/bugsnag/android/internal/ImmutableConfig;
-	public static synthetic fun copy$default (Lcom/bugsnag/android/internal/ImmutableConfig;Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIILkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;ILjava/lang/Object;)Lcom/bugsnag/android/internal/ImmutableConfig;
+	public final fun copy (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)Lcom/bugsnag/android/internal/ImmutableConfig;
+	public static synthetic fun copy$default (Lcom/bugsnag/android/internal/ImmutableConfig;Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;ILjava/lang/Object;)Lcom/bugsnag/android/internal/ImmutableConfig;
 	public fun equals (Ljava/lang/Object;)Z
 	public final fun getApiKey ()Ljava/lang/String;
 	public final fun getAppInfo ()Landroid/content/pm/ApplicationInfo;
@@ -922,6 +925,7 @@ public final class com/bugsnag/android/internal/ImmutableConfig {
 	public final fun getSendLaunchCrashesSynchronously ()Z
 	public final fun getSendThreads ()Lcom/bugsnag/android/ThreadSendPolicy;
 	public final fun getTelemetry ()Ljava/util/Set;
+	public final fun getThreadCollectionTimeLimitMillis ()J
 	public final fun getVersionCode ()Ljava/lang/Integer;
 	public fun hashCode ()I
 	public final fun shouldDiscardBreadcrumb (Lcom/bugsnag/android/BreadcrumbType;)Z

bugsnag-android-core/detekt-baseline.xml

@@ -19,7 +19,7 @@
     <ID>LongParameterList:EventStorageModule.kt$EventStorageModule$( contextModule: ContextModule, configModule: ConfigModule, dataCollectionModule: DataCollectionModule, bgTaskService: BackgroundTaskService, trackerModule: TrackerModule, systemServiceModule: SystemServiceModule, notifier: Notifier, callbackState: CallbackState )</ID>
     <ID>LongParameterList:NativeStackframe.kt$NativeStackframe$( /** * The name of the method that was being executed */ var method: String?, /** * The location of the source file */ var file: String?, /** * The line number within the source file this stackframe refers to */ var lineNumber: Number?, /** * The address of the instruction where the event occurred. */ var frameAddress: Long?, /** * The address of the function where the event occurred. */ var symbolAddress: Long?, /** * The address of the library where the event occurred. */ var loadAddress: Long?, /** * Whether this frame identifies the program counter */ var isPC: Boolean?, /** * The type of the error */ var type: ErrorType? = null, /** * Identifies the exact build this frame originates from. */ var codeIdentifier: String? = null, )</ID>
     <ID>LongParameterList:StateEvent.kt$StateEvent.Install$( @JvmField val apiKey: String, @JvmField val autoDetectNdkCrashes: Boolean, @JvmField val appVersion: String?, @JvmField val buildUuid: String?, @JvmField val releaseStage: String?, @JvmField val lastRunInfoPath: String, @JvmField val consecutiveLaunchCrashes: Int, @JvmField val sendThreads: ThreadSendPolicy )</ID>
-    <ID>LongParameterList:ThreadState.kt$ThreadState$( allThreads: List&lt;JavaThread>, currentThread: JavaThread, exc: Throwable?, isUnhandled: Boolean, maxThreadCount: Int, projectPackages: Collection&lt;String>, logger: Logger )</ID>
+    <ID>LongParameterList:ThreadState.kt$ThreadState$( allThreads: List&lt;JavaThread>, currentThread: JavaThread, exc: Throwable?, isUnhandled: Boolean, maxThreadCount: Int, threadCollectionTimeLimitMillis: Long, projectPackages: Collection&lt;String>, logger: Logger )</ID>
     <ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$299</ID>
     <ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$429</ID>
     <ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$499</ID>
@@ -65,6 +65,7 @@
     <ID>TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware</ID>
     <ID>TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector</ID>
     <ID>TooManyFunctions:EventInternal.kt$EventInternal : FeatureFlagAwareStreamableMetadataAwareUserAware</ID>
+    <ID>UnusedPrivateProperty:ManifestConfigLoader.kt$ManifestConfigLoader.Companion$private const val LAUNCH_CRASH_THRESHOLD_MS = "$BUGSNAG_NS.LAUNCH_CRASH_THRESHOLD_MS"</ID>
     <ID>UnusedPrivateProperty:ThreadStateTest.kt$ThreadStateTest$private val configuration = generateImmutableConfig()</ID>
     <ID>UseCheckOrError:BackgroundTaskServiceTest.kt$BackgroundTaskServiceTest$throw IllegalStateException()</ID>
     <ID>UseCheckOrError:BugsnagEventMapper.kt$BugsnagEventMapper$throw IllegalStateException("cannot find json property '$key'")</ID>

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ManifestConfigLoaderTest.kt

@@ -50,6 +50,7 @@ class ManifestConfigLoaderTest {
             assertEquals(maxPersistedEvents, 32)
             assertEquals(maxPersistedSessions, 128)
             assertEquals(maxReportedThreads, 200)
+            assertEquals(threadCollectionTimeLimitMillis, 5000)
             assertTrue(sendLaunchCrashesSynchronously)
             assertEquals(launchDurationMillis, 5000)
             assertEquals("android", appType)
@@ -87,6 +88,7 @@ class ManifestConfigLoaderTest {
             putInt("com.bugsnag.android.MAX_PERSISTED_EVENTS", 52)
             putInt("com.bugsnag.android.MAX_PERSISTED_SESSIONS", 64)
             putInt("com.bugsnag.android.MAX_REPORTED_THREADS", 100)
+            putLong("com.bugsnag.android.THREAD_COLLECTION_TIME_LIMIT_MS", 300)
             putInt("com.bugsnag.android.LAUNCH_DURATION_MILLIS", 7000)
             putBoolean("com.bugsnag.android.SEND_LAUNCH_CRASHES_SYNCHRONOUSLY", false)
             putString("com.bugsnag.android.APP_TYPE", "react-native")
@@ -126,6 +128,8 @@ class ManifestConfigLoaderTest {
             assertEquals(maxPersistedEvents, 52)
             assertEquals(maxPersistedSessions, 64)
             assertEquals(maxReportedThreads, 100)
+            assertEquals(threadCollectionTimeLimitMillis, 300)
+            @Suppress("DEPRECATION")
             assertEquals(launchDurationMillis, 7000)
             assertFalse(sendLaunchCrashesSynchronously)
             assertEquals("react-native", appType)

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt

@@ -21,6 +21,7 @@ class ThreadStateTest {
         null,
         true,
         1000,
+        1000,
         ThreadSendPolicy.ALWAYS,
         Collections.emptyList(),
         NoopLogger,
@@ -64,6 +65,7 @@ class ThreadStateTest {
             trace,
             true,
             1000,
+            1000,
             ThreadSendPolicy.ALWAYS,
             Collections.emptyList(),
             NoopLogger,
@@ -90,6 +92,7 @@ class ThreadStateTest {
             trace,
             true,
             1000,
+            1000,
             ThreadSendPolicy.ALWAYS,
             Collections.emptyList(),
             NoopLogger,
@@ -114,6 +117,7 @@ class ThreadStateTest {
             trace,
             true,
             1000,
+            1000,
             ThreadSendPolicy.ALWAYS,
             Collections.emptyList(),
             NoopLogger,
@@ -159,6 +163,7 @@ class ThreadStateTest {
             exc,
             true,
             1000,
+            1000,
             ThreadSendPolicy.ALWAYS,
             Collections.emptyList(),
             NoopLogger,
@@ -196,6 +201,7 @@ class ThreadStateTest {
             trace,
             true,
             2,
+            1000,
             ThreadSendPolicy.ALWAYS,
             Collections.emptyList(),
             NoopLogger,
@@ -224,6 +230,7 @@ class ThreadStateTest {
             exc,
             true,
             4,
+            1000,
             ThreadSendPolicy.ALWAYS,
             Collections.emptyList(),
             NoopLogger,
@@ -251,6 +258,7 @@ class ThreadStateTest {
             trace,
             true,
             1000,
+            1000,
             ThreadSendPolicy.NEVER,
             Collections.emptyList(),
             NoopLogger,
@@ -262,6 +270,26 @@ class ThreadStateTest {
         assertEquals(0, json.length())
     }
 
+    @Test
+    fun testThreadCollectionTimeLimitMillis() {
+        val currentThread = Thread.currentThread()
+        val allThreads = allThreads()
+        val state = ThreadState(
+            trace,
+            true,
+            1000,
+            0,
+            ThreadSendPolicy.ALWAYS,
+            Collections.emptyList(),
+            NoopLogger,
+            currentThread,
+            allThreads
+        )
+        val json = streamableToJsonArray(state)
+
+        assertEquals(1, json.length())
+    }
+
     private fun verifyCurrentThreadStructure(
         json: JSONArray,
         currentThreadId: Long,

bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt

@@ -43,6 +43,7 @@ internal class ConfigInternal(
     var maxPersistedEvents: Int = DEFAULT_MAX_PERSISTED_EVENTS
     var maxPersistedSessions: Int = DEFAULT_MAX_PERSISTED_SESSIONS
     var maxReportedThreads: Int = DEFAULT_MAX_REPORTED_THREADS
+    var threadCollectionTimeLimitMillis: Long = DEFAULT_THREAD_COLLECTION_TIME_LIMIT_MS
     var maxStringValueLength: Int = DEFAULT_MAX_STRING_VALUE_LENGTH
     var context: String? = null
 
@@ -139,6 +140,8 @@ internal class ConfigInternal(
                 "maxPersistedSessions" to maxPersistedSessions else null,
             if (maxReportedThreads != defaultConfig.maxReportedThreads)
                 "maxReportedThreads" to maxReportedThreads else null,
+            if (threadCollectionTimeLimitMillis != defaultConfig.threadCollectionTimeLimitMillis)
+                "threadCollectionTimeLimitMillis" to threadCollectionTimeLimitMillis else null,
             if (persistenceDirectory != null)
                 "persistenceDirectorySet" to true else null,
             if (sendThreads != defaultConfig.sendThreads)
@@ -153,6 +156,7 @@ internal class ConfigInternal(
         private const val DEFAULT_MAX_PERSISTED_SESSIONS = 128
         private const val DEFAULT_MAX_PERSISTED_EVENTS = 32
         private const val DEFAULT_MAX_REPORTED_THREADS = 200
+        private const val DEFAULT_THREAD_COLLECTION_TIME_LIMIT_MS: Long = 5000
         private const val DEFAULT_LAUNCH_CRASH_THRESHOLD_MS: Long = 5000
         private const val DEFAULT_MAX_STRING_VALUE_LENGTH = 10000
 

bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java

@@ -490,8 +490,8 @@ public void setMaxBreadcrumbs(@IntRange(from = 0, to = 500) int maxBreadcrumbs)
      *
      * By default, 32 events are persisted.
      */
-    public int getMaxPersistedEvents() { 
-        return impl.getMaxPersistedEvents(); 
+    public int getMaxPersistedEvents() {
+        return impl.getMaxPersistedEvents();
     }
 
     /**
@@ -536,6 +536,31 @@ public void setMaxReportedThreads(@IntRange(from = 0) int maxReportedThreads) {
         }
     }
 
+
+    /**
+     * Gets the maximum time for collecting threads and traces.
+     * By default, up to 200 threads are reported.
+     */
+    public long getThreadCollectionTimeLimitMillis() {
+        return impl.getThreadCollectionTimeLimitMillis();
+    }
+
+    /**
+     * Sets the maximum time for collecting threads and traces.
+     * By default, up to 500 milliseconds are reported.
+     */
+    public void setThreadCollectionTimeLimitMillis(
+            @IntRange(from = 0) long threadCollectionTimeLimitMillis
+    ) {
+        if (threadCollectionTimeLimitMillis >= 0) {
+            impl.setThreadCollectionTimeLimitMillis(threadCollectionTimeLimitMillis);
+        } else {
+            getLogger().e("Invalid configuration value detected. "
+                    + "Option threadCollectionTimeLimitMillis should be a positive integer."
+                    + "Supplied value is " + threadCollectionTimeLimitMillis);
+        }
+    }
+
     /**
      * Sets the maximum number of persisted sessions which will be stored. Once the threshold is
      * reached, the oldest session will be deleted.
@@ -1113,7 +1138,7 @@ public void setAttemptDeliveryOnCrash(boolean attemptDeliveryOnCrash) {
 
     /**
      * Whether Bugsnag should try to send crashing errors prior to app termination.
-     * 
+     *
      * @see #setAttemptDeliveryOnCrash(boolean)
      */
     public boolean isAttemptDeliveryOnCrash() {

bugsnag-android-core/src/main/java/com/bugsnag/android/ManifestConfigLoader.kt

@@ -39,6 +39,8 @@ internal class ManifestConfigLoader {
         private const val MAX_PERSISTED_EVENTS = "$BUGSNAG_NS.MAX_PERSISTED_EVENTS"
         private const val MAX_PERSISTED_SESSIONS = "$BUGSNAG_NS.MAX_PERSISTED_SESSIONS"
         private const val MAX_REPORTED_THREADS = "$BUGSNAG_NS.MAX_REPORTED_THREADS"
+        private const val THREAD_COLLECTION_TIME_LIMIT_MS = "$BUGSNAG_NS.THREAD_COLLECTION_TIME_LIMIT_MS"
+        private const val LAUNCH_CRASH_THRESHOLD_MS = "$BUGSNAG_NS.LAUNCH_CRASH_THRESHOLD_MS"
         private const val LAUNCH_DURATION_MILLIS = "$BUGSNAG_NS.LAUNCH_DURATION_MILLIS"
         private const val SEND_LAUNCH_CRASHES_SYNCHRONOUSLY = "$BUGSNAG_NS.SEND_LAUNCH_CRASHES_SYNCHRONOUSLY"
         private const val APP_TYPE = "$BUGSNAG_NS.APP_TYPE"
@@ -80,6 +82,10 @@ internal class ManifestConfigLoader {
                 maxPersistedEvents = data.getInt(MAX_PERSISTED_EVENTS, maxPersistedEvents)
                 maxPersistedSessions = data.getInt(MAX_PERSISTED_SESSIONS, maxPersistedSessions)
                 maxReportedThreads = data.getInt(MAX_REPORTED_THREADS, maxReportedThreads)
+                threadCollectionTimeLimitMillis = data.getLong(
+                    THREAD_COLLECTION_TIME_LIMIT_MS,
+                    threadCollectionTimeLimitMillis
+                )
                 launchDurationMillis = data.getInt(
                     LAUNCH_DURATION_MILLIS,
                     launchDurationMillis.toInt()