Skip to content

Commit

Permalink
Merge pull request #853 from bugsnag/karl/unity-tweaks
Browse files Browse the repository at this point in the history
Basic LoadedImages caching. Also renamed StackTrace to PayloadStackTrace so that the name doesn't clash with the system StackTrace.
  • Loading branch information
kstenerud authored Nov 14, 2024
2 parents bd1060d + 3207bea commit 07985b4
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 12 deletions.
8 changes: 8 additions & 0 deletions Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/LoadedImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ class LoadedImages
/// </remarks>
public void Refresh()
{
UInt64 loadedNativeImagesAt = NativeCode.bugsnag_lastChangedLoadedImages();
if (loadedNativeImagesAt == lastLoadedNativeImagesAt)
{
return;
}
lastLoadedNativeImagesAt = loadedNativeImagesAt;

// Ask for the current count * 2 in case new images get added between calls
var nativeImages = new NativeLoadedImage[NativeCode.bugsnag_getLoadedImageCount() * 2];
var count = NativeCode.bugsnag_getLoadedImages(nativeImages, (UInt64)nativeImages.LongLength);
Expand Down Expand Up @@ -54,6 +61,7 @@ public LoadedImage FindImageAtAddress(UInt64 address)
/// The currently loaded images, as of the last call to Refresh().
/// </summary>
public LoadedImage[] Images;
private UInt64 lastLoadedNativeImagesAt = 0;

private class AddressToImageComparator : IComparer
{
Expand Down
7 changes: 7 additions & 0 deletions Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ partial class NativeCode
[DllImport(Import)]
internal static extern UInt64 bugsnag_getLoadedImages([In, Out] NativeLoadedImage[] images, UInt64 capacity);

/// <summary>
/// Marker for the last time the loaded images list was changed. If this value changes,
/// the loaded images have been changed.
/// </summary>
[DllImport(Import)]
internal static extern UInt64 bugsnag_lastChangedLoadedImages();

/// <summary>
/// Unlock the mutex protecting the loaded images list.
/// </summary>
Expand Down
16 changes: 8 additions & 8 deletions Bugsnag/Assets/Bugsnag/Runtime/Payload/ErrorBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ internal Error FromUnityLogMessage(UnityLogMessage logMessage, System.Diagnostic
{
var match = Regex.Match(logMessage.Condition, ERROR_CLASS_MESSAGE_PATTERN, RegexOptions.Singleline);

var lines = new StackTrace(logMessage.StackTrace).StackTraceLines;
var lines = new PayloadStackTrace(logMessage.StackTrace).StackTraceLines;
if (lines.Length == 0)
{
lines = new StackTrace(fallbackStackFrames).StackTraceLines;
lines = new PayloadStackTrace(fallbackStackFrames).StackTraceLines;
}

var handledState = forceUnhandled
Expand All @@ -67,7 +67,7 @@ internal Error FromUnityLogMessage(UnityLogMessage logMessage, System.Diagnostic
var androidErrorData = ProcessAndroidError(message);
errorClass = androidErrorData[0];
message = androidErrorData[1];
lines = new StackTrace(logMessage.StackTrace, StackTraceFormat.AndroidJava).StackTraceLines;
lines = new PayloadStackTrace(logMessage.StackTrace, StackTraceFormat.AndroidJava).StackTraceLines;
handledState = HandledState.ForUnhandledException();
}
return new Error(errorClass, message, lines, handledState, isAndroidJavaException);
Expand All @@ -81,14 +81,14 @@ internal Error FromUnityLogMessage(UnityLogMessage logMessage, System.Diagnostic

internal Error FromStringInfo(string name, string message, string stacktrace)
{
var stackFrames = new StackTrace(stacktrace).StackTraceLines;
var stackFrames = new PayloadStackTrace(stacktrace).StackTraceLines;
return new Error(name, message, stackFrames);
}

internal Error FromSystemException(System.Exception exception, string stackTrace)
{
var errorClass = exception.GetType().Name;
var lines = new StackTrace(stackTrace).StackTraceLines;
var lines = new PayloadStackTrace(stackTrace).StackTraceLines;

return new Error(errorClass, exception.Message, lines);
}
Expand All @@ -103,19 +103,19 @@ internal Error FromSystemException(System.Exception exception, System.Diagnostic
var androidErrorData = ProcessAndroidError(exception.Message);
var androidErrorClass = androidErrorData[0];
var androidErrorMessage = androidErrorData[1];
var lines = new StackTrace(exception.StackTrace, StackTraceFormat.AndroidJava).StackTraceLines;
var lines = new PayloadStackTrace(exception.StackTrace, StackTraceFormat.AndroidJava).StackTraceLines;
return new Error(androidErrorClass, androidErrorMessage, lines, HandledState.ForUnhandledException(), true);
}
else
{
StackTraceLine[] lines;
if (!string.IsNullOrEmpty(exception.StackTrace))
{
lines = new StackTrace(exception.StackTrace).StackTraceLines;
lines = new PayloadStackTrace(exception.StackTrace).StackTraceLines;
}
else
{
lines = new StackTrace(alternativeStackTrace).StackTraceLines;
lines = new PayloadStackTrace(alternativeStackTrace).StackTraceLines;
}
return new Error(errorClass, exception.Message, lines);
}
Expand Down
8 changes: 4 additions & 4 deletions Bugsnag/Assets/Bugsnag/Runtime/Payload/StackTraceLine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ internal enum StackTraceFormat { Standard, AndroidJava };
/// Represents a set of Bugsnag payload stacktrace lines that are generated from a single StackTrace provided
/// by the runtime.
/// </summary>
class StackTrace : IEnumerable<StackTraceLine>
class PayloadStackTrace : IEnumerable<StackTraceLine>
{
public StackTraceLine[] StackTraceLines { get; private set; }

internal StackTrace(StackFrame[] stackFrames)
internal PayloadStackTrace(StackFrame[] stackFrames)
{
StackTraceLines = new StackTraceLine[stackFrames.Length];
for (int i = 0; i < stackFrames.Length; i++)
Expand All @@ -27,9 +27,9 @@ internal StackTrace(StackFrame[] stackFrames)
}
}

internal StackTrace(string stackTrace) : this(stackTrace, StackTraceFormat.Standard) { }
internal PayloadStackTrace(string stackTrace) : this(stackTrace, StackTraceFormat.Standard) { }

internal StackTrace(string stackTrace, StackTraceFormat format)
internal PayloadStackTrace(string stackTrace, StackTraceFormat format)
{
string[] lines = stackTrace.Split(new[] {"\r\n","\r","\n", System.Environment.NewLine },
System.StringSplitOptions.RemoveEmptyEntries);
Expand Down
7 changes: 7 additions & 0 deletions Bugsnag/NativeSrc/Cocoa/BugsnagUnity.mm
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ bool isValid() const {
// All currently loaded images. This MUST be kept ordered: LoadAddress low to high.
static std::vector<NativeLoadedImage> allImages;
static std::mutex allImagesMutex;
static uint64_t lastChangedLoadedImages;

static void add_image(const struct mach_header *header, intptr_t slide) {
NativeLoadedImage image(header);
Expand All @@ -132,6 +133,7 @@ static void add_image(const struct mach_header *header, intptr_t slide) {
std::lock_guard<std::mutex> guard(allImagesMutex);
allImages.push_back(std::move(image));
std::sort(allImages.begin(), allImages.end());
lastChangedLoadedImages = CFAbsoluteTimeGetCurrent() * 1000000000ULL;
}

static void remove_image(const struct mach_header *header, intptr_t slide) {
Expand All @@ -144,6 +146,7 @@ static void remove_image(const struct mach_header *header, intptr_t slide) {
if (foundImage != allImages.end()) {
allImages.erase(foundImage);
}
lastChangedLoadedImages = CFAbsoluteTimeGetCurrent() * 1000000000ULL;
}

static void register_for_dyld_changes(void) {
Expand All @@ -163,6 +166,10 @@ uint64_t bugsnag_getLoadedImageCount() {
return allImages.size();
}

uint64_t bugsnag_lastChangedLoadedImages() {
return lastChangedLoadedImages;
}

uint64_t bugsnag_getLoadedImages(NativeLoadedImage *images, uint64_t capacity) {
// Lock and hold the lock until bugsnag_unlockLoadedImages() is called.
// We do this to allow the C# side time to copy over FileName and UUID.
Expand Down

0 comments on commit 07985b4

Please sign in to comment.