fix: Remove dispatch queue metadata collection to fix crash (#3522)

* Remove dispatch queue metadata collection

* Hardcode "main" as the name of the main thread

* Format code

* Update CHANGELOG.md

* use thread wrapper main thread dispatch

---------

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Co-authored-by: Andrew McKnight <andrew.mcknight@sentry.io>

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixes
+
+- Remove dispatch queue metadata collection to fix crash (#3522)
+
 ### Features
 
 - Send debug meta for app start transactions (#3543)

SentryTestUtils/SentryProfilerMocks.h

@@ -10,8 +10,7 @@ using namespace sentry::profiling;
 NS_ASSUME_NONNULL_BEGIN
 
 Backtrace mockBacktrace(thread::TIDType threadID, const int threadPriority,
- const char *_Nullable threadName, std::uint64_t queueAddress, std::string queueLabel,
- std::vector<std::uintptr_t> addresses);
+ const char *_Nullable threadName, std::vector<std::uintptr_t> addresses);
 
 NS_ASSUME_NONNULL_END
 

SentryTestUtils/SentryProfilerMocks.mm

@@ -4,7 +4,7 @@
 
 Backtrace
 mockBacktrace(thread::TIDType threadID, const int threadPriority, const char *threadName,
- std::uint64_t queueAddress, std::string queueLabel, std::vector<std::uintptr_t> addresses)
+ std::vector<std::uintptr_t> addresses)
 {
  ThreadMetadata threadMetadata;
  if (threadName != nullptr) {
@@ -13,13 +13,8 @@
  threadMetadata.threadID = threadID;
  threadMetadata.priority = threadPriority;
 
- QueueMetadata queueMetadata;
- queueMetadata.address = queueAddress;
- queueMetadata.label = std::make_shared<std::string>(queueLabel);
-
  Backtrace backtrace;
  backtrace.threadMetadata = threadMetadata;
- backtrace.queueMetadata = queueMetadata;
  backtrace.addresses = std::vector<std::uintptr_t>(addresses);
 
  return backtrace;

SentryTestUtils/SentryProfilerMocksSwiftCompatible.h

@@ -17,8 +17,6 @@ NS_ASSUME_NONNULL_BEGIN
  threadID:(uint64_t)threadID
  threadPriority:(const int)threadPriority
  threadName:(nullable NSString *)threadName
- queueAddress:(uint64_t)queueAddress
- queueLabel:(NSString *)queueLabel
  addresses:(NSArray<NSNumber *> *)addresses;
 
 @end

SentryTestUtils/SentryProfilerMocksSwiftCompatible.mm

@@ -16,8 +16,6 @@ + (void)appendMockBacktraceToState:(SentryProfilerState *)state
  threadID:(uint64_t)threadID
  threadPriority:(const int)threadPriority
  threadName:(nullable NSString *)threadName
- queueAddress:(uint64_t)queueAddress
- queueLabel:(NSString *)queueLabel
  addresses:(NSArray<NSNumber *> *)addresses
 {
  auto backtraceAddresses = std::vector<std::uintptr_t>();
@@ -27,8 +25,7 @@ + (void)appendMockBacktraceToState:(SentryProfilerState *)state
  }
 
  auto backtrace = mockBacktrace(threadID, threadPriority,
- [threadName cStringUsingEncoding:NSUTF8StringEncoding], queueAddress,
- [queueLabel cStringUsingEncoding:NSUTF8StringEncoding], backtraceAddresses);
+ [threadName cStringUsingEncoding:NSUTF8StringEncoding], backtraceAddresses);
  backtrace.absoluteTimestamp = SentryDependencyContainer.sharedInstance.dateProvider.systemTime;
  [state appendBacktrace:backtrace];
 }

Sources/Sentry/Profiling/SentryProfilerState.mm

@@ -4,6 +4,9 @@
 # import "SentryFormatter.h"
 # import "SentryProfileTimeseries.h"
 # import "SentrySample.h"
+# import "SentryThreadWrapper.h"
+# import <mach/mach_types.h>
+# import <mach/port.h>
 # import <mutex>
 
 # if defined(DEBUG)
@@ -42,7 +45,6 @@ - (instancetype)init
  _stacks = [NSMutableArray<NSArray<NSNumber *> *> array];
  _frames = [NSMutableArray<NSDictionary<NSString *, id> *> array];
  _threadMetadata = [NSMutableDictionary<NSString *, NSMutableDictionary *> dictionary];
- _queueMetadata = [NSMutableDictionary<NSString *, NSDictionary *> dictionary];
  _frameIndexLookup = [NSMutableDictionary<NSString *, NSNumber *> dictionary];
  _stackIndexLookup = [NSMutableDictionary<NSString *, NSNumber *> dictionary];
  }
@@ -54,12 +56,15 @@ - (instancetype)init
 @implementation SentryProfilerState {
  SentryProfilerMutableState *_mutableState;
  std::mutex _lock;
+ thread_t _mainThreadID;
 }
 
 - (instancetype)init
 {
  if (self = [super init]) {
  _mutableState = [[SentryProfilerMutableState alloc] init];
+ _mainThreadID = 0;
+ [SentryThreadWrapper onMainThread:^{ [self cacheMainThreadID]; }];
  }
  return self;
 }
@@ -71,36 +76,38 @@ - (void)mutate:(void (^)(SentryProfilerMutableState *))block
  block(_mutableState);
 }
 
+- (void)cacheMainThreadID
+{
+ std::lock_guard<std::mutex> l(_lock);
+ NSAssert([NSThread isMainThread], @"Must be called on main thread");
+ const auto currentThread = pthread_mach_thread_np(pthread_self());
+ if (MACH_PORT_VALID(currentThread)) {
+ _mainThreadID = currentThread;
+ }
+}
+
 - (void)appendBacktrace:(const Backtrace &)backtrace
 {
  [self mutate:^(SentryProfilerMutableState *state) {
  const auto threadID = sentry_stringForUInt64(backtrace.threadMetadata.threadID);
 
- NSString *queueAddress = nil;
- if (backtrace.queueMetadata.address != 0) {
- queueAddress = sentry_formatHexAddressUInt64(backtrace.queueMetadata.address);
- }
  NSMutableDictionary<NSString *, id> *metadata = state.threadMetadata[threadID];
  if (metadata == nil) {
  metadata = [NSMutableDictionary<NSString *, id> dictionary];
  state.threadMetadata[threadID] = metadata;
  }
- if (!backtrace.threadMetadata.name.empty() && metadata[@"name"] == nil) {
- metadata[@"name"] =
- [NSString stringWithUTF8String:backtrace.threadMetadata.name.c_str()];
+ if (metadata[@"name"] == nil) {
+ if (!backtrace.threadMetadata.name.empty()) {
+ metadata[@"name"] =
+ [NSString stringWithUTF8String:backtrace.threadMetadata.name.c_str()];
+ } else if (self->_mainThreadID != 0
+ && backtrace.threadMetadata.threadID == self->_mainThreadID) {
+ metadata[@"name"] = @"main";
+ }
  }
  if (backtrace.threadMetadata.priority != -1 && metadata[@"priority"] == nil) {
  metadata[@"priority"] = @(backtrace.threadMetadata.priority);
  }
- if (queueAddress != nil && state.queueMetadata[queueAddress] == nil
- && backtrace.queueMetadata.label != nullptr) {
- NSString *const labelNSStr =
- [NSString stringWithUTF8String:backtrace.queueMetadata.label->c_str()];
- // -[NSString stringWithUTF8String:] can return `nil` for malformed string data
- if (labelNSStr != nil) {
- state.queueMetadata[queueAddress] = @ { @"label" : labelNSStr };
- }
- }
 # if defined(DEBUG)
  const auto symbols
  = backtrace_symbols(reinterpret_cast<void *const *>(backtrace.addresses.data()),
@@ -136,9 +143,6 @@ - (void)appendBacktrace:(const Backtrace &)backtrace
  const auto sample = [[SentrySample alloc] init];
  sample.absoluteTimestamp = backtrace.absoluteTimestamp;
  sample.threadID = backtrace.threadMetadata.threadID;
- if (queueAddress != nil) {
- sample.queueAddress = queueAddress;
- }
 
  const auto stackKey = [stack componentsJoinedByString:@"|"];
  const auto stackIndex = state.stackIndexLookup[stackKey];
@@ -162,8 +166,6 @@ - (void)appendBacktrace:(const Backtrace &)backtrace
  NSMutableArray<SentrySample *> *const samples = [_mutableState.samples copy];
  NSMutableArray<NSArray<NSNumber *> *> *const stacks = [_mutableState.stacks copy];
  NSMutableArray<NSDictionary<NSString *, id> *> *const frames = [_mutableState.frames copy];
- NSMutableDictionary<NSString *, NSDictionary *> *const queueMetadata =
- [_mutableState.queueMetadata copy];
 
  // thread metadata contains a mutable substructure, so it's not enough to perform a copy of
  // the top-level dictionary, we need to go deeper to copy the mutable subdictionaries
@@ -177,7 +179,6 @@ - (void)appendBacktrace:(const Backtrace &)backtrace
  @"stacks" : stacks,
  @"frames" : frames,
  @"thread_metadata" : threadMetadata,
- @"queue_metadata" : queueMetadata
  }
  };
 }

Sources/Sentry/SentryBacktrace.cpp

@@ -159,38 +159,7 @@ namespace profiling {
  const auto depth = backtrace(*thread, *pair.second, addresses, stackBounds,
  &reachedEndOfStack, kMaxBacktraceDepth, 0);
 
- // Retrieving queue metadata *must* be done after suspending the thread,
- // because otherwise the queue could be deallocated in the middle of us
- // trying to read from it. This doesn't use any of the pthread APIs, only
- // thread_info, so the above comment about the deadlock does not apply here.
- const auto queueAddress = thread->dispatchQueueAddress();
- if (queueAddress != 0) {
- // This operation is read-only and does not result in any heap allocations.
- auto cachedMetadata = cache->metadataForQueue(queueAddress);
-
- // Copy the queue label onto the stack to avoid a heap allocation.
- char newQueueLabel[256];
- *newQueueLabel = '\0';
- if (cachedMetadata.address == 0) {
- // There's no cached metadata, so we should try to read it.
- const auto queue = reinterpret_cast<dispatch_queue_t *>(queueAddress);
- const auto queueLabel = dispatch_queue_get_label(*queue);
- strlcpy(newQueueLabel, queueLabel, sizeof(newQueueLabel));
- }
-
- thread->resume();
-
- if (cachedMetadata.address == 0) {
- cachedMetadata.address = queueAddress;
- // These cause heap allocations but it's safe now since the thread has
- // been resumed above.
- cachedMetadata.label = std::make_shared<std::string>(newQueueLabel);
- cache->setQueueMetadata(cachedMetadata);
- }
- bt.queueMetadata = std::move(cachedMetadata);
- } else {
- thread->resume();
- }
+ thread->resume();
 
  // ############################################
  // END DEADLOCK WARNING

Sources/Sentry/SentryThreadHandle.cpp

@@ -10,13 +10,6 @@
 # include <mach/mach.h>
 # include <pthread.h>
 
-/**
- * SentryCrashMemory.h uses the restrict keyword, which is valid in C99 but
- * invalid in C++; we can use __restrict as an alternative.
- * */
-# define restrict __restrict
-# include "SentryCrashMemory.h"
-
 namespace sentry {
 namespace profiling {
 
@@ -123,27 +116,6 @@ namespace profiling {
  return {};
  }
 
- std::uint64_t
- ThreadHandle::dispatchQueueAddress() const noexcept
- {
- if (handle_ == THREAD_NULL) {
- return {};
- }
- integer_t infoBuffer[THREAD_IDENTIFIER_INFO_COUNT] = { 0 };
- thread_info_t info = infoBuffer;
- mach_msg_type_number_t count = THREAD_IDENTIFIER_INFO_COUNT;
- const auto idInfo = reinterpret_cast<thread_identifier_info_t>(info);
- if (thread_info(handle_, THREAD_IDENTIFIER_INFO, info, &count) == KERN_SUCCESS
- && sentrycrashmem_isMemoryReadable(idInfo, sizeof(*idInfo))) {
- const auto queuePtr = reinterpret_cast<dispatch_queue_t *>(idInfo->dispatch_qaddr);
- if (queuePtr != nullptr && sentrycrashmem_isMemoryReadable(queuePtr, sizeof(*queuePtr))
- && idInfo->thread_handle != 0 && *queuePtr != nullptr) {
- return idInfo->dispatch_qaddr;
- }
- }
- return 0;
- }
-
  int
  ThreadHandle::priority() const noexcept
  {

Sources/Sentry/SentryThreadMetadataCache.cpp

@@ -61,28 +61,6 @@ namespace profiling {
  return (*it).metadata;
  }
  }
-
- QueueMetadata
- ThreadMetadataCache::metadataForQueue(std::uint64_t address) const
- {
- if (address == 0) {
- return {};
- }
- const auto it = std::find_if(queueMetadataCache_.cbegin(), queueMetadataCache_.cend(),
- [address](const QueueMetadata &metadata) { return metadata.address == address; });
- if (it == queueMetadataCache_.cend()) {
- return { 0, {} };
- } else {
- return (*it);
- }
- }
-
- void
- ThreadMetadataCache::setQueueMetadata(QueueMetadata metadata)
- {
- queueMetadataCache_.push_back(std::move(metadata));
- }
-
 } // namespace profiling
 } // namespace sentry
 

Sources/Sentry/include/SentryBacktrace.hpp

@@ -21,7 +21,6 @@ namespace profiling {
 
  struct Backtrace {
  ThreadMetadata threadMetadata;
- QueueMetadata queueMetadata;
  std::uint64_t absoluteTimestamp;
  std::vector<std::uintptr_t> addresses;
  };

Sources/Sentry/include/SentryProfilerState.h

@@ -31,8 +31,6 @@ NSString *parseBacktraceSymbolsFunctionName(const char *symbol);
 @property (nonatomic, strong, readonly) NSMutableArray<NSDictionary<NSString *, id> *> *frames;
 @property (nonatomic, strong, readonly)
  NSMutableDictionary<NSString *, NSMutableDictionary *> *threadMetadata;
-@property (nonatomic, strong, readonly)
- NSMutableDictionary<NSString *, NSDictionary *> *queueMetadata;
 
 /*
  * Maintain an index of unique frames to avoid duplicating large amounts of data. Every

Sources/Sentry/include/SentryThreadHandle.hpp

@@ -97,17 +97,6 @@ namespace profiling {
  */
  std::string name() const noexcept;
 
- /**
- * @return The address of the dispatch queue currently associated with
- * the thread, or 0 if there is no valid queue. This function checks if
- * the memory at the returned address is readable, but there is no guarantee
- * that the queue will continue to stay valid by the time you deference it,
- * as queue deallocation can happen concurrently.
- *
- * @warning This function is not async-signal safe!
- */
- std::uint64_t dispatchQueueAddress() const noexcept;
-
  /**
  * @return The priority of the specified thread, or -1 if the thread priority
  * could not be successfully determined.

Sources/Sentry/include/SentryThreadMetadataCache.hpp

@@ -18,13 +18,6 @@ namespace profiling {
  int priority;
  };
 
- struct QueueMetadata {
- std::uint64_t address;
- // std::string always heap allocates a string buffer, since this data structure
- // might be created while its unsafe to allocate, we wrap it in a pointer.
- std::shared_ptr<std::string> label;
- };
-
  /**
  * Caches thread and queue metadata (name, priority, etc.) for reuse while profiling,
  * since querying that metadata every time can be expensive.
@@ -42,21 +35,6 @@ namespace profiling {
  */
  ThreadMetadata metadataForThread(const ThreadHandle &thread);
 
- /**
- * Returns the metadata for the queue at the specified address.
- * @param address The address of the queue.
- * @return @c QueueMetadata for the queue at the specified address if a cached
- * entry exists, or an empty @c QueueMetadata with the address set to 0 if it
- * does not exist.
- */
- QueueMetadata metadataForQueue(std::uint64_t address) const;
-
- /**
- * Stores metadata for the queue at the address specified in `metadata.address`
- * @param metadata The metadata to associate with the queue.
- */
- void setQueueMetadata(QueueMetadata metadata);
-
  ThreadMetadataCache() = default;
  ThreadMetadataCache(const ThreadMetadataCache &) = delete;
  ThreadMetadataCache &operator=(const ThreadMetadataCache &) = delete;
@@ -67,7 +45,6 @@ namespace profiling {
  ThreadMetadata metadata;
  };
  std::vector<const ThreadHandleMetadataPair> threadMetadataCache_;
- std::vector<const QueueMetadata> queueMetadataCache_;
  };
 
 } // namespace profiling