Skip to content

fix: Record cached dyld images async #7269

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.

Sign up for GitHub

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

Draft
noahsmartin wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: Record cached dyld images async #7269

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
189 changes: 62 additions & 127 deletions Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <mach-o/dyld_images.h>
#include <pthread.h>
#include <stdio.h>
#include <dispatch/dispatch.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
Expand Down Expand Up @@ -60,94 +61,64 @@ sentry_resetFuncForAddRemoveImage(void)
# define _will_add_image()
#endif // defined(SENTRY_TEST) || defined(SENTRY_TEST_CI) || defined(DEBUG)

typedef struct SentryCrashBinaryImageNode {
SentryCrashBinaryImage image;
bool available;
struct SentryCrashBinaryImageNode *next;
} SentryCrashBinaryImageNode;
#include <stdatomic.h>

static SentryCrashBinaryImageNode rootNode = { 0 };
static SentryCrashBinaryImageNode *tailNode = NULL;
static pthread_mutex_t binaryImagesMutex = PTHREAD_MUTEX_INITIALIZER;
#define MAX_DYLD_IMAGES 4096

static sentrycrashbic_cacheChangeCallback imageAddedCallback = NULL;
static sentrycrashbic_cacheChangeCallback imageRemovedCallback = NULL;
typedef struct {
_Atomic(uint32_t) ready; // 0 = not published, 1 = published
SentryCrashBinaryImage image;
} PublishedBinaryImage;

static void
binaryImageAdded(const struct mach_header *header, intptr_t slide)
{
pthread_mutex_lock(&binaryImagesMutex);
if (tailNode == NULL) {
pthread_mutex_unlock(&binaryImagesMutex);
static PublishedBinaryImage g_images[MAX_DYLD_IMAGES];
static _Atomic(uint32_t) g_next_index = 0;
static _Atomic(uint32_t) g_overflowed = 0;

static void add_dyld_image(const struct mach_header* mh) {
uint32_t idx =
atomic_fetch_add_explicit(&g_next_index, 1, memory_order_relaxed);

if (idx >= MAX_DYLD_IMAGES) {
atomic_store_explicit(&g_overflowed, 1, memory_order_relaxed);
return;
}
pthread_mutex_unlock(&binaryImagesMutex);

Dl_info info;
if (!dladdr(header, &info) || info.dli_fname == NULL) {
if (!dladdr(mh, &info) || info.dli_fname == NULL) {
return;
}

SentryCrashBinaryImage binaryImage = { 0 };
if (!sentrycrashdl_getBinaryImageForHeader(
(const void *)header, info.dli_fname, &binaryImage, false)) {
return;
}
PublishedBinaryImage* entry = &g_images[idx];
sentrycrashdl_getBinaryImageForHeader(mh, info.dli_fname, &entry->image, false);

SentryCrashBinaryImageNode *newNode = malloc(sizeof(SentryCrashBinaryImageNode));
newNode->available = true;
newNode->image = binaryImage;
newNode->next = NULL;
_will_add_image();
pthread_mutex_lock(&binaryImagesMutex);
// Recheck tailNode as it could be null when
// stopped from another thread.
if (tailNode != NULL) {
tailNode->next = newNode;
tailNode = tailNode->next;
} else {
free(newNode);
newNode = NULL;
}
pthread_mutex_unlock(&binaryImagesMutex);
if (newNode && imageAddedCallback) {
imageAddedCallback(&newNode->image);
}
// ---- Publish ----
atomic_store_explicit(&entry->ready, 1, memory_order_release);
}

static void
binaryImageRemoved(const struct mach_header *header, intptr_t slide)
{
SentryCrashBinaryImageNode *nextNode = &rootNode;

while (nextNode != NULL) {
if (nextNode->image.address == (uint64_t)header) {
nextNode->available = false;
if (imageRemovedCallback) {
imageRemovedCallback(&nextNode->image);
}
break;
}
nextNode = nextNode->next;
}
static void dyld_add_image_cb(const struct mach_header* mh, intptr_t slide) {
add_dyld_image(mh);
}

void dyld_tracker_start(void) {
Copy link
Contributor

@itaybre itaybre Jan 23, 2026

Choose a reason for hiding this comment

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

Maybe this should have sentry's prefix

sentry_dyld_register_func_for_add_image(dyld_add_image_cb);
}

void
sentrycrashbic_iterateOverImages(sentrycrashbic_imageIteratorCallback callback, void *context)
{
/**
We can't use locks here because this is meant to be used during crashes,
where we can't use async unsafe functions. In order to avoid potential problems,
we choose an approach that doesn't remove nodes from the list.
*/
SentryCrashBinaryImageNode *nextNode = &rootNode;

// If tailNode is null it means the cache was stopped, therefore we end the iteration.
// This will minimize any race condition effect without the need for locks.
while (nextNode != NULL && tailNode != NULL) {
if (nextNode->available) {
callback(&nextNode->image, context);
uint32_t count =
atomic_load_explicit(&g_next_index, memory_order_acquire);

if (count > MAX_DYLD_IMAGES) count = MAX_DYLD_IMAGES;

for (uint32_t i = 0; i < count; i++) {
PublishedBinaryImage* src = &g_images[i];

if (!atomic_load_explicit(&src->ready, memory_order_acquire)) {
return; // stop at first unpublished entry
}
nextNode = nextNode->next;

callback(&src->image, context);
}
}

Expand All @@ -170,93 +141,57 @@ sentrycrashbic_shouldAddDyld(void)

// Since Apple no longer includes dyld in the images listed `_dyld_image_count` and related
// functions We manually include it to our cache.
SentryCrashBinaryImageNode *
sentrycrashbic_getDyldNode(void)
void
sentrycrashbic_addDyldNode(void)
{
const struct mach_header *header = sentryDyldHeader;

SentryCrashBinaryImage binaryImage = { 0 };
if (!sentrycrashdl_getBinaryImageForHeader((const void *)header, "dyld", &binaryImage, false)) {
return NULL;
return;
}

SentryCrashBinaryImageNode *newNode = malloc(sizeof(SentryCrashBinaryImageNode));
newNode->available = true;
newNode->image = binaryImage;
newNode->next = NULL;

return newNode;
add_dyld_image(header);
}

void
sentrycrashbic_startCache(void)
{
pthread_mutex_lock(&binaryImagesMutex);
if (tailNode != NULL) {
// Already initialized
pthread_mutex_unlock(&binaryImagesMutex);
return;
}

if (sentrycrashbic_shouldAddDyld()) {
sentrycrashdl_initialize();
SentryCrashBinaryImageNode *dyldNode = sentrycrashbic_getDyldNode();
tailNode = dyldNode;
rootNode.next = dyldNode;
} else {
tailNode = &rootNode;
rootNode.next = NULL;
sentrycrashbic_addDyldNode();
}
pthread_mutex_unlock(&binaryImagesMutex);

// During a call to _dyld_register_func_for_add_image() the callback func is called for every
// existing image
sentry_dyld_register_func_for_add_image(&binaryImageAdded);
sentry_dyld_register_func_for_remove_image(&binaryImageRemoved);
// This must be done on a background thread to not block app launch due to the extensive use of locks
// in the image added callback. The main culprit is the calls to `dladdr`.
// The downside of doing this async is if there is a crash very shortly after app launch we might
// not have recorded all the load addresses of images yet. We think this is an acceptible tradeoff
// to not block app launch, since it's always possible to crash early in app launch before
// Sentry can capture the crash.
//
// A future update to this code could record everything but the filename synchronously, and then
// add in the image name async. However, that would still block app launch somewhat and it's not
// clear that is worth the benefits.
dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{
dyld_tracker_start();
});
}

void
sentrycrashbic_stopCache(void)
{
pthread_mutex_lock(&binaryImagesMutex);
if (tailNode == NULL) {
pthread_mutex_unlock(&binaryImagesMutex);
return;
}

SentryCrashBinaryImageNode *node = rootNode.next;
rootNode.next = NULL;
tailNode = NULL;

while (node != NULL) {
SentryCrashBinaryImageNode *nextNode = node->next;
free(node);
node = nextNode;
}

pthread_mutex_unlock(&binaryImagesMutex);
}

static void
initialReportToCallback(SentryCrashBinaryImage *image, void *context)
{
sentrycrashbic_cacheChangeCallback callback = (sentrycrashbic_cacheChangeCallback)context;
callback(image);
// TODO: Why do we need to support stopping it?
}

void
sentrycrashbic_registerAddedCallback(sentrycrashbic_cacheChangeCallback callback)
{
imageAddedCallback = callback;
if (callback) {
pthread_mutex_lock(&binaryImagesMutex);
sentrycrashbic_iterateOverImages(&initialReportToCallback, callback);
pthread_mutex_unlock(&binaryImagesMutex);
}
// TODO: I think this is only for when we used to have on-device symbolication. Should be able to remove now
}

void
sentrycrashbic_registerRemovedCallback(sentrycrashbic_cacheChangeCallback callback)
{
imageRemovedCallback = callback;
// TODO: I think this is only for when we used to have on-device symbolication. Should be able to remove now
}
Loading