-
Notifications
You must be signed in to change notification settings - Fork 35
Background thread support #67
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| using System; | ||
| using System.Collections; | ||
| using System.Collections.Generic; | ||
| using System.Threading; | ||
| using UnityEngine; | ||
|
|
||
| [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Backtrace.Unity.Tests.Runtime")] | ||
|
|
@@ -20,14 +21,16 @@ public class BacktraceClient : MonoBehaviour, IBacktraceClient | |
| { | ||
| public BacktraceConfiguration Configuration; | ||
|
|
||
| public const string VERSION = "3.3.3"; | ||
| public const string VERSION = "3.3.4"; | ||
| public bool Enabled { get; private set; } | ||
|
|
||
| /// <summary> | ||
| /// Client attributes | ||
| /// </summary> | ||
| private readonly Dictionary<string, string> _clientAttributes = new Dictionary<string, string>(); | ||
|
|
||
| internal readonly Stack<BacktraceReport> BackgroundExceptions = new Stack<BacktraceReport>(); | ||
|
|
||
| /// <summary> | ||
| /// Attribute object accessor | ||
| /// </summary> | ||
|
|
@@ -352,7 +355,7 @@ public void Refresh() | |
| } | ||
|
|
||
| Enabled = true; | ||
|
|
||
| _current = Thread.CurrentThread; | ||
| CaptureUnityMessages(); | ||
| _reportLimitWatcher = new ReportLimitWatcher(Convert.ToUInt32(Configuration.ReportPerMin)); | ||
|
|
||
|
|
@@ -413,15 +416,28 @@ private void Awake() | |
| /// <summary> | ||
| /// Update native client internal ANR timer. | ||
| /// </summary> | ||
| private void Update() | ||
| private void LateUpdate() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To give main thread to update before an anr thread There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah okay that makes sense |
||
| { | ||
| _nativeClient?.UpdateClientTime(Time.unscaledTime); | ||
|
|
||
| if (BackgroundExceptions.Count == 0) | ||
| { | ||
| return; | ||
| } | ||
| while (BackgroundExceptions.Count > 0) | ||
| { | ||
| // use SendReport method isntead of Send method | ||
| // because we already applied all watchdog/skipReport rules | ||
| // so we don't need to apply them once again | ||
| SendReport(BackgroundExceptions.Pop()); | ||
| } | ||
| } | ||
|
|
||
| private void OnDestroy() | ||
| { | ||
| Enabled = false; | ||
| Application.logMessageReceived -= HandleUnityMessage; | ||
| Application.logMessageReceivedThreaded -= HandleUnityBackgroundException; | ||
| #if UNITY_ANDROID || UNITY_IOS | ||
| Application.lowMemory -= HandleLowMemory; | ||
| _nativeClient?.Disable(); | ||
|
|
@@ -691,6 +707,8 @@ internal void OnAnrDetected(string stackTrace) | |
| } | ||
| #endif | ||
|
|
||
| private Thread _current; | ||
|
|
||
| /// <summary> | ||
| /// Handle Unity unhandled exceptions | ||
| /// </summary> | ||
|
|
@@ -700,12 +718,24 @@ private void CaptureUnityMessages() | |
| if (Configuration.HandleUnhandledExceptions || Configuration.NumberOfLogs != 0) | ||
| { | ||
| Application.logMessageReceived += HandleUnityMessage; | ||
| Application.logMessageReceivedThreaded += HandleUnityBackgroundException; | ||
| #if UNITY_ANDROID || UNITY_IOS | ||
| Application.lowMemory += HandleLowMemory; | ||
| #endif | ||
| } | ||
| } | ||
|
|
||
| internal void HandleUnityBackgroundException(string message, string stackTrace, LogType type) | ||
| { | ||
| // validate if a message is from main thread | ||
| // and skip messages from main thread | ||
| if (Thread.CurrentThread == _current) | ||
| { | ||
| return; | ||
| } | ||
| HandleUnityMessage(message, stackTrace, type); | ||
| } | ||
|
|
||
| #if UNITY_ANDROID || UNITY_IOS | ||
| internal void HandleLowMemory() | ||
| { | ||
|
|
@@ -824,10 +854,22 @@ private bool ShouldSendReport(Exception exception, List<string> attachmentPaths, | |
| { | ||
| return false; | ||
| } | ||
|
|
||
| //check rate limiting | ||
| bool shouldProcess = _reportLimitWatcher.WatchReport(new DateTime().Timestamp()); | ||
| if (shouldProcess) | ||
| { | ||
| // This condition checks if we should send exception from current thread | ||
| // if comparision result confirm that we're trying to send an exception from different | ||
| // thread than main, we should add the exception object to the exception list | ||
| // and let update method send data to Backtrace. | ||
| if (Thread.CurrentThread.ManagedThreadId != _current.ManagedThreadId) | ||
| { | ||
| var report = new BacktraceReport(exception, attributes, attachmentPaths); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be repeated 3 times. Can we pull this into a function?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really because of different parameters that this function require and cost of BacktraceReport creation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay I see. Since this gets called in Update method we need to consider cost of BacktraceReport creation. |
||
| report.Attributes["exception.thread"] = Thread.CurrentThread.ManagedThreadId.ToString(); | ||
| BackgroundExceptions.Push(report); | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| if (OnClientReportLimitReached != null) | ||
|
|
@@ -852,6 +894,17 @@ private bool ShouldSendReport(string message, List<string> attachmentPaths, Dict | |
| bool shouldProcess = _reportLimitWatcher.WatchReport(new DateTime().Timestamp()); | ||
| if (shouldProcess) | ||
| { | ||
| // This condition checks if we should send exception from current thread | ||
| // if comparision result confirm that we're trying to send an exception from different | ||
| // thread than main, we should add the exception object to the exception list | ||
| // and let update method send data to Backtrace. | ||
| if (Thread.CurrentThread.ManagedThreadId != _current.ManagedThreadId) | ||
| { | ||
| var report = new BacktraceReport(message, attributes, attachmentPaths); | ||
| report.Attributes["exception.thread"] = Thread.CurrentThread.ManagedThreadId.ToString(); | ||
| BackgroundExceptions.Push(report); | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| if (OnClientReportLimitReached != null) | ||
|
|
@@ -880,6 +933,16 @@ private bool ShouldSendReport(BacktraceReport report) | |
| bool shouldProcess = _reportLimitWatcher.WatchReport(new DateTime().Timestamp()); | ||
| if (shouldProcess) | ||
| { | ||
| // This condition checks if we should send exception from current thread | ||
| // if comparision result confirm that we're trying to send an exception from different | ||
| // thread than main, we should add the exception object to the exception list | ||
| // and let update method send data to Backtrace. | ||
| if (Thread.CurrentThread.ManagedThreadId != _current.ManagedThreadId) | ||
| { | ||
| report.Attributes["exception.thread"] = Thread.CurrentThread.ManagedThreadId.ToString(); | ||
| BackgroundExceptions.Push(report); | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| if (OnClientReportLimitReached != null) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| using Backtrace.Unity.Model; | ||
| using Backtrace.Unity.Types; | ||
| using NUnit.Framework; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Threading; | ||
|
|
||
| namespace Backtrace.Unity.Tests.Runtime | ||
| { | ||
| public class BackgroundThreadSupport : BacktraceBaseTest | ||
| { | ||
| [SetUp] | ||
| public void Setup() | ||
| { | ||
| BeforeSetup(); | ||
| BacktraceClient.Configuration = GetBasicConfiguration(); | ||
| AfterSetup(); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_BackgroundExceptionShouldntThrow_ExceptionIsSavedInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
| var mainThreadId = Thread.CurrentThread.ManagedThreadId; | ||
| var thread = new Thread(() => | ||
| { | ||
| Assert.IsTrue(Thread.CurrentThread.ManagedThreadId != mainThreadId); | ||
| var exception = new InvalidOperationException(exceptionMessage); | ||
| client.Send(exception); | ||
| }); | ||
| thread.Start(); | ||
| thread.Join(); | ||
|
|
||
| Assert.IsNotEmpty(BacktraceClient.BackgroundExceptions); | ||
| Assert.AreEqual(exceptionMessage, BacktraceClient.BackgroundExceptions.First().Message); | ||
| Assert.IsTrue(BacktraceClient.BackgroundExceptions.First().ExceptionTypeReport); | ||
| } | ||
|
|
||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_BackgroundReportShouldntThrow_ExceptionIsSavedInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
| var mainThreadId = Thread.CurrentThread.ManagedThreadId; | ||
| var thread = new Thread(() => | ||
| { | ||
| Assert.IsTrue(Thread.CurrentThread.ManagedThreadId != mainThreadId); | ||
| var exception = new InvalidOperationException(exceptionMessage); | ||
| client.Send(new BacktraceReport(exception)); | ||
| }); | ||
| thread.Start(); | ||
| thread.Join(); | ||
|
|
||
| Assert.IsNotEmpty(BacktraceClient.BackgroundExceptions); | ||
| Assert.AreEqual(exceptionMessage, BacktraceClient.BackgroundExceptions.First().Message); | ||
| Assert.IsTrue(BacktraceClient.BackgroundExceptions.First().ExceptionTypeReport); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_BackgroundReportWithAttributesShouldntThrow_ExceptionIsSavedInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
| var attributeKey = "attribute-key"; | ||
| var attributeValue = exceptionMessage; | ||
| var mainThreadId = Thread.CurrentThread.ManagedThreadId; | ||
| var thread = new Thread(() => | ||
| { | ||
| Assert.IsTrue(Thread.CurrentThread.ManagedThreadId != mainThreadId); | ||
| var exception = new InvalidOperationException(exceptionMessage); | ||
| client.Send(new BacktraceReport(exception, new Dictionary<string, string> { { attributeKey, attributeValue } })); | ||
| }); | ||
| thread.Start(); | ||
| thread.Join(); | ||
|
|
||
| Assert.IsNotEmpty(BacktraceClient.BackgroundExceptions); | ||
| var storedReport = BacktraceClient.BackgroundExceptions.First(); | ||
| Assert.AreEqual(exceptionMessage, storedReport.Message); | ||
| Assert.IsTrue(storedReport.ExceptionTypeReport); | ||
| Assert.IsNotNull(storedReport.Attributes[attributeKey]); | ||
| Assert.AreEqual(storedReport.Attributes[attributeKey], attributeValue); | ||
| } | ||
|
|
||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_BackgroundMessageShouldntThrow_ExceptionIsSavedInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
| var mainThreadId = Thread.CurrentThread.ManagedThreadId; | ||
| var thread = new Thread(() => | ||
| { | ||
| Assert.IsTrue(Thread.CurrentThread.ManagedThreadId != mainThreadId); | ||
| client.Send(exceptionMessage); | ||
| }); | ||
| thread.Start(); | ||
| thread.Join(); | ||
|
|
||
| Assert.IsNotEmpty(BacktraceClient.BackgroundExceptions); | ||
| Assert.AreEqual(exceptionMessage, BacktraceClient.BackgroundExceptions.First().Message); | ||
| Assert.IsFalse(BacktraceClient.BackgroundExceptions.First().ExceptionTypeReport); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_BackgroundUnhandledExceptionShouldntThrow_ExceptionIsSavedInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
| var mainThreadId = Thread.CurrentThread.ManagedThreadId; | ||
|
|
||
| var thread = new Thread(() => | ||
| { | ||
| Assert.IsTrue(Thread.CurrentThread.ManagedThreadId != mainThreadId); | ||
| client.HandleUnityBackgroundException(exceptionMessage, string.Empty, UnityEngine.LogType.Exception); | ||
| }); | ||
| thread.Start(); | ||
| thread.Join(); | ||
|
|
||
| Assert.IsNotEmpty(BacktraceClient.BackgroundExceptions); | ||
| Assert.AreEqual(exceptionMessage, BacktraceClient.BackgroundExceptions.First().Message); | ||
| Assert.IsTrue(BacktraceClient.BackgroundExceptions.First().ExceptionTypeReport); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_UserShouldBeAbleToFilterUnhandledExceptions_ReportShouldntBeAvailableInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
|
|
||
| BacktraceClient.SkipReport = (ReportFilterType type, Exception e, string message) => | ||
| { | ||
| return true; | ||
| }; | ||
| var mainThreadId = Thread.CurrentThread.ManagedThreadId; | ||
| var thread = new Thread(() => | ||
| { | ||
| Assert.IsTrue(Thread.CurrentThread.ManagedThreadId != mainThreadId); | ||
| client.HandleUnityBackgroundException(exceptionMessage, string.Empty, UnityEngine.LogType.Exception); | ||
| }); | ||
| thread.Start(); | ||
| thread.Join(); | ||
|
|
||
| Assert.IsEmpty(BacktraceClient.BackgroundExceptions); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_UserShouldBeAbleToFilterReports_ReportShouldntBeAvailableInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
|
|
||
| BacktraceClient.SkipReport = (ReportFilterType type, Exception e, string message) => | ||
| { | ||
| return true; | ||
| }; | ||
| var thread = new Thread(() => | ||
| { | ||
| client.Send(exceptionMessage); | ||
| var exception = new InvalidOperationException(exceptionMessage); | ||
| client.Send(exception); | ||
| client.Send(new BacktraceReport(exception)); | ||
| }); | ||
|
|
||
| thread.Start(); | ||
| thread.Join(); | ||
|
|
||
| Assert.IsEmpty(BacktraceClient.BackgroundExceptions); | ||
| } | ||
|
|
||
|
|
||
| [Test] | ||
| public void TestBackgroundThreadSupport_RateLimitSkipReports_ReportShouldntBeAvailableInMainThreadLoop() | ||
| { | ||
| var client = BacktraceClient; | ||
| string exceptionMessage = "foo"; | ||
|
|
||
| uint rateLimit = 5; | ||
| var expectedNumberOfSkippedReports = 5; | ||
| int actualNumberOfSkippedReports = 0; | ||
|
|
||
| client.SetClientReportLimit(rateLimit); | ||
| client.OnClientReportLimitReached = (BacktraceReport report) => | ||
| { | ||
| actualNumberOfSkippedReports++; | ||
| }; | ||
|
|
||
| var thread = new Thread(() => | ||
| { | ||
| for (int i = 0; i < rateLimit + expectedNumberOfSkippedReports; i++) | ||
| { | ||
| client.Send(new InvalidOperationException(exceptionMessage)); | ||
| } | ||
|
|
||
| }); | ||
| thread.Start(); | ||
| thread.Join(); | ||
| Assert.IsNotEmpty(BacktraceClient.BackgroundExceptions); | ||
| Assert.AreEqual(rateLimit, BacktraceClient.BackgroundExceptions.Count); | ||
| Assert.AreEqual(expectedNumberOfSkippedReports, actualNumberOfSkippedReports); | ||
| } | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this Stack instead of Queue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yes, you're right. Queue makes more sense here