Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 5 additions & 0 deletions src/backends/sentry_backend_native.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ static HANDLE g_ipc_mutex = NULL;
#elif defined(SENTRY_PLATFORM_MACOS)
// macOS uses a plain pthread mutex instead of named semaphores (sem_open)
// because App Sandbox blocks POSIX named semaphores.
# ifdef SENTRY__MUTEX_INIT_DYN
SENTRY__MUTEX_INIT_DYN(g_ipc_sync_mutex)
# else
static sentry_mutex_t g_ipc_sync_mutex = SENTRY__MUTEX_INIT;
# endif
#else
# include <semaphore.h>
static sem_t *g_ipc_init_sem = SEM_FAILED;
Expand Down Expand Up @@ -250,6 +254,7 @@ native_backend_startup(
#elif defined(SENTRY_PLATFORM_IOS)
state->ipc = sentry__crash_ipc_init_app(NULL);
#elif defined(SENTRY_PLATFORM_MACOS)
SENTRY__MUTEX_INIT_DYN_ONCE(g_ipc_sync_mutex);
state->ipc = sentry__crash_ipc_init_app(&g_ipc_sync_mutex);
#else
state->ipc = sentry__crash_ipc_init_app(g_ipc_init_sem);
Expand Down
8 changes: 8 additions & 0 deletions src/modulefinder/sentry_modulefinder_apple.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ typedef struct segment_command_64 mach_segment_command_type;
#endif

static bool g_initialized = false;
#ifdef SENTRY__MUTEX_INIT_DYN
SENTRY__MUTEX_INIT_DYN(g_mutex)
#else
static sentry_mutex_t g_mutex = SENTRY__MUTEX_INIT;
#endif
static sentry_value_t g_modules = { 0 };

static void
Expand Down Expand Up @@ -77,6 +81,7 @@ add_image(const struct mach_header *mh, intptr_t UNUSED(vmaddr_slide))
}
}

SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex);
sentry__mutex_lock(&g_mutex);

sentry_value_t modules = g_modules;
Expand All @@ -98,6 +103,7 @@ remove_image(const struct mach_header *mh, intptr_t UNUSED(vmaddr_slide))
return;
}

SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex);
sentry__mutex_lock(&g_mutex);

if (sentry_value_is_null(g_modules)
Expand Down Expand Up @@ -135,6 +141,7 @@ sentry_get_modules_list(void)
// `add_image` callback). We do that because we have observed deadlocks when
// code concurrently `dlopen`s and thus invokes the `add_image` callback
// from a different thread.
SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex);
sentry__mutex_lock(&g_mutex);
if (!g_initialized) {
g_modules = sentry_value_new_list();
Expand Down Expand Up @@ -162,6 +169,7 @@ sentry_get_modules_list(void)
void
sentry_clear_modulecache(void)
{
SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex);
sentry__mutex_lock(&g_mutex);
sentry_value_decref(g_modules);
g_modules = sentry_value_new_null();
Expand Down
6 changes: 6 additions & 0 deletions src/sentry_app_hang_monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ sentry__app_hang_monitor_set_stackwalk_fn(sentry__app_hang_stackwalk_fn fn)

static bool g_running = false;
static sentry_threadid_t g_thread;
# ifdef SENTRY__MUTEX_INIT_DYN
SENTRY__MUTEX_INIT_DYN(g_wait_mutex)
# else
static sentry_mutex_t g_wait_mutex = SENTRY__MUTEX_INIT;
# endif
static sentry_cond_t g_wait_cond;
static uint64_t g_timeout_ms = 0;

Expand Down Expand Up @@ -127,6 +131,7 @@ sentry__app_hang_monitor_start(const sentry_options_t *options)
return 0;
}

SENTRY__MUTEX_INIT_DYN_ONCE(g_wait_mutex);
g_timeout_ms = options->app_hang_timeout;
sentry__cond_init(&g_wait_cond);
// Arm before spawning: the worker uses is_active() as its run condition, so
Expand All @@ -150,6 +155,7 @@ sentry__app_hang_monitor_stop(void)
return;
}
sentry__app_hang_set_active(false);
SENTRY__MUTEX_INIT_DYN_ONCE(g_wait_mutex);
sentry__mutex_lock(&g_wait_mutex);
sentry__cond_wake(&g_wait_cond);
sentry__mutex_unlock(&g_wait_mutex);
Expand Down
78 changes: 54 additions & 24 deletions src/sentry_scope.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,9 @@
#endif

static bool g_scope_initialized = false;
static bool g_scope_flush_pending = false;
static sentry_scope_t g_scope = { 0 };
#ifdef SENTRY__MUTEX_INIT_DYN
SENTRY__MUTEX_INIT_DYN(g_lock)
#else
static sentry_mutex_t g_lock = SENTRY__MUTEX_INIT;
#endif
SENTRY__RWLOCK_INIT_DYN(g_lock)

static sentry_value_t
get_client_sdk(void)
Expand Down Expand Up @@ -133,43 +130,76 @@ cleanup_scope(sentry_scope_t *scope)
void
sentry__scope_cleanup(void)
{
SENTRY__MUTEX_INIT_DYN_ONCE(g_lock);
sentry__mutex_lock(&g_lock);
SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock);
sentry__rwlock_write_lock(&g_lock);
if (g_scope_initialized) {
g_scope_initialized = false;
g_scope_flush_pending = false;
cleanup_scope(&g_scope);
}
sentry__mutex_unlock(&g_lock);
sentry__rwlock_unlock(&g_lock);
}

sentry_scope_t *
sentry__scope_lock(void)
const sentry_scope_t *
sentry__scope_read_lock(void)
{
SENTRY__MUTEX_INIT_DYN_ONCE(g_lock);
sentry__mutex_lock(&g_lock);
return get_scope();
SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock);
for (;;) {
sentry__rwlock_read_lock(&g_lock);
if (g_scope_initialized) {
return &g_scope;
}
sentry__rwlock_unlock(&g_lock);

sentry_scope_t *scope = sentry__scope_write_lock();
(void)scope;
sentry__scope_write_unlock();
}
}

void
sentry__scope_unlock(void)
sentry__scope_read_unlock(void)
{
SENTRY__MUTEX_INIT_DYN_ONCE(g_lock);
sentry__mutex_unlock(&g_lock);
SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock);
sentry__rwlock_unlock(&g_lock);
}

void
sentry__scope_flush_unlock(void)
{
sentry__scope_unlock();
SENTRY_WITH_OPTIONS (options) {
// we try to unlock the scope as soon as possible. The
// backend will do its own `WITH_SCOPE` internally.
if (options->backend && options->backend->flush_scope_func) {
options->backend->flush_scope_func(options->backend, options);
sentry__scope_write_unlock(void)
{
SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock);
bool should_flush
= g_scope_flush_pending && sentry__rwlock_write_depth(&g_lock) == 1;
if (should_flush) {
g_scope_flush_pending = false;
}
bool released_outermost = sentry__rwlock_unlock(&g_lock);
if (released_outermost && should_flush) {
SENTRY_WITH_OPTIONS (options) {
// we try to unlock the scope as soon as possible. The
// backend will do its own `WITH_SCOPE` internally.
if (options->backend && options->backend->flush_scope_func) {
options->backend->flush_scope_func(options->backend, options);
}
}
}
}

sentry_scope_t *
sentry__scope_write_lock(void)
{
SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock);
sentry__rwlock_write_lock(&g_lock);
return get_scope();
}

void
sentry__scope_flush_write_unlock(void)
{
g_scope_flush_pending = true;
sentry__scope_write_unlock();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NO_FLUSH can still flush

Medium Severity

sentry__scope_write_unlock flushes whenever g_scope_flush_pending is set on the outermost unlock. SENTRY_WITH_SCOPE_MUT_NO_FLUSH uses that path, so an inner SENTRY_WITH_SCOPE_MUT can force a flush on the outer NO_FLUSH exit. That breaks the no-flush contract used by paths like sentry_add_breadcrumb.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 495cd85. Configure here.


sentry_scope_t *
sentry_scope_new(void)
{
Expand Down
32 changes: 21 additions & 11 deletions src/sentry_scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,24 @@ typedef enum {
} sentry_scope_mode_t;

/**
* This will acquire a lock on the global scope.
* This will acquire a read lock on the global scope.
*/
sentry_scope_t *sentry__scope_lock(void);
const sentry_scope_t *sentry__scope_read_lock(void);

/**
* Release the lock on the global scope.
* Release the read lock on the global scope.
*/
void sentry__scope_unlock(void);
void sentry__scope_read_unlock(void);

/**
* This will acquire a write lock on the global scope.
*/
sentry_scope_t *sentry__scope_write_lock(void);

/**
* Release the write lock on the global scope.
*/
void sentry__scope_write_unlock(void);

/**
* This will free all the data attached to the global scope
Expand All @@ -81,7 +91,7 @@ void sentry__scope_cleanup(void);
* This function must be called while holding the scope lock, and it will be
* unlocked internally.
*/
void sentry__scope_flush_unlock(void);
void sentry__scope_flush_write_unlock(void);

/**
* This will merge the requested data which is in the given `scope` to the given
Expand Down Expand Up @@ -118,14 +128,14 @@ void sentry__scope_remove_attribute_n(
* inside a code block.
*/
#define SENTRY_WITH_SCOPE(Scope) \
for (const sentry_scope_t *Scope = sentry__scope_lock(); Scope; \
sentry__scope_unlock(), Scope = NULL)
for (const sentry_scope_t *Scope = sentry__scope_read_lock(); Scope; \
sentry__scope_read_unlock(), Scope = NULL)
#define SENTRY_WITH_SCOPE_MUT(Scope) \
for (sentry_scope_t *Scope = sentry__scope_lock(); Scope; \
sentry__scope_flush_unlock(), Scope = NULL)
for (sentry_scope_t *Scope = sentry__scope_write_lock(); Scope; \
sentry__scope_flush_write_unlock(), Scope = NULL)
#define SENTRY_WITH_SCOPE_MUT_NO_FLUSH(Scope) \
for (sentry_scope_t *Scope = sentry__scope_lock(); Scope; \
sentry__scope_unlock(), Scope = NULL)
for (sentry_scope_t *Scope = sentry__scope_write_lock(); Scope; \
sentry__scope_write_unlock(), Scope = NULL)

/**
* Rebuilds the scope's dynamic sampling context (DSC) from the SDK options
Expand Down
Loading
Loading