Skip to content

Commit d1f65ac

Browse files
committed
Fix cache reset thread safety and benchmark test name mappings
Acquire exclusive access via atomic_exchange before resetting cache storage in ksdl_resetCache() and ksbic_resetCache(). This follows the same lock-free pattern used by the lookup functions. Also update workflow benchmark test names to match actual test names and add missing cache miss variants.
1 parent e342ea2 commit d1f65ac

File tree

3 files changed

+35
-9
lines changed

3 files changed

+35
-9
lines changed

.github/workflows/benchmarks.yml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,13 @@ jobs:
174174
"BinaryImageForHeaderAll": "Binary image info (all)",
175175
"TypicalCrashSymbolication": "Typical crash symbolication",
176176
"FullCrashReport": "Full crash report generation",
177-
"RepeatedSameAddressLookup": "Repeated same address lookup",
178-
"SequentialNearbyAddresses": "Sequential nearby addresses",
179-
"ExactFunctionAddressLookup": "Exact function address lookup",
180-
"NonExactAddressLookup": "Non-exact address lookup",
177+
"RepeatedLookupCacheHit": "Repeated lookup (cache hit)",
178+
"RepeatedLookupCacheMiss": "Repeated lookup (cache miss)",
179+
"SequentialNearbyAddressesCacheHit": "Sequential nearby addresses (cache hit)",
180+
"ExactFunctionLookupCacheHit": "Exact function lookup (cache hit)",
181+
"ExactFunctionLookupCacheMiss": "Exact function lookup (cache miss)",
182+
"NonExactLookupCacheHit": "Non-exact lookup (cache hit)",
183+
"NonExactLookupCacheMiss": "Non-exact lookup (cache miss)",
181184
"IsMemoryReadableSmall": "Check readable (64B)",
182185
"IsMemoryReadablePage": "Check readable (4KB)",
183186
"IsMemoryReadableLarge": "Check readable (64KB)",

Sources/KSCrashRecordingCore/KSBinaryImageCache.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,19 @@ const ks_dyld_image_info *ksbic_getImages(uint32_t *count)
253253
void ksbic_resetCache(void)
254254
{
255255
g_all_image_infos = NULL;
256-
g_cache_storage.count = 0;
257-
atomic_store(&g_cache_ptr, &g_cache_storage);
256+
257+
// Acquire exclusive access to the cache before resetting
258+
KSBinaryImageRangeCache *cache = atomic_exchange(&g_cache_ptr, NULL);
259+
if (cache != NULL) {
260+
cache->count = 0;
261+
atomic_store(&g_cache_ptr, cache);
262+
} else {
263+
// Cache is in use by another thread - reset storage directly
264+
// and restore pointer (the other thread will see stale data but
265+
// that's acceptable for a reset operation)
266+
g_cache_storage.count = 0;
267+
atomic_store(&g_cache_ptr, &g_cache_storage);
268+
}
258269
}
259270

260271
const struct mach_header *ksbic_findImageForAddress(uintptr_t address, uintptr_t *outSlide, const char **outName)

Sources/KSCrashRecordingCore/KSDynamicLinker.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,22 @@ void ksdl_init(void)
9090

9191
void ksdl_resetCache(void)
9292
{
93-
// Reset both caches and allow re-initialization
93+
// Reset binary image cache first (it has its own locking)
9494
ksbic_resetCache();
95-
g_symbol_cache_storage.count = 0;
96-
atomic_store(&g_symbol_cache_ptr, &g_symbol_cache_storage);
95+
96+
// Acquire exclusive access to the symbol cache before resetting
97+
KSSymbolCache *cache = atomic_exchange(&g_symbol_cache_ptr, NULL);
98+
if (cache != NULL) {
99+
cache->count = 0;
100+
atomic_store(&g_symbol_cache_ptr, cache);
101+
} else {
102+
// Cache is in use by another thread - reset storage directly
103+
// and restore pointer (the other thread will see stale data but
104+
// that's acceptable for a reset operation)
105+
g_symbol_cache_storage.count = 0;
106+
atomic_store(&g_symbol_cache_ptr, &g_symbol_cache_storage);
107+
}
108+
97109
atomic_store(&g_initialized, false);
98110
}
99111

0 commit comments

Comments
 (0)