Skip to content

Commit 1ade4e0

Browse files
jpnurmiclaude
andauthored
fix(batcher): fix deadlock on reinit (#1518)
* test(logs): add reinit deadlock test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(metrics): add reinit deadlock test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): eliminate g_options_lock from flush path to prevent deadlock When sentry_init() is called while a batcher thread is mid-flush, a deadlock occurs: the main thread holds g_options_lock and waits for the batcher thread to join, while the batcher thread tries to acquire g_options_lock via SENTRY_WITH_OPTIONS during flush. Store the options pointer in the batcher at startup and use it directly during flush instead of going through SENTRY_WITH_OPTIONS. Add sentry__envelope_new_with_dsn() to create envelopes without locking, and sentry__options_get_user_consent() for lock-free consent checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ref(batcher): store individual fields instead of options pointer Replace the raw options pointer with individual fields (dsn, transport, run, user_consent) to avoid unsynchronized access to options members from the batcher thread. The dsn is incref'd, and user_consent is a pointer to the atomic field (NULL when consent is not required). Revert sentry__options_get_user_consent since it is no longer needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add changelog entry for batcher deadlock fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(envelope): hold DSN ref across sentry__envelope_new_with_dsn call The DSN pointer was extracted under g_options_lock but used after the lock was released, racing with sentry_close freeing it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): enable metrics instead of logs in metrics_reinit test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): release DSN ref when thread spawn fails Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 76fbfd6 commit 1ade4e0

13 files changed

Lines changed: 120 additions & 27 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
**Fixes**:
6+
7+
- Fix deadlock when re-initializing the SDK while logs or metrics threads are mid-flush. ([#1518](https://github.com/getsentry/sentry-native/pull/1518))
8+
39
## 0.12.7
410

511
**Features**:

src/sentry_batcher.c

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#include "sentry_batcher.h"
2-
#include "sentry_core.h"
32
#include "sentry_cpu_relax.h"
43
#include "sentry_options.h"
54

@@ -116,19 +115,22 @@ sentry__batcher_flush(sentry_batcher_t *batcher, bool crash_safe)
116115
}
117116
sentry_value_set_by_key(logs, "items", log_items);
118117

119-
sentry_envelope_t *envelope = sentry__envelope_new();
118+
sentry_envelope_t *envelope
119+
= sentry__envelope_new_with_dsn(batcher->dsn);
120120
batcher->batch_func(envelope, logs);
121121

122-
SENTRY_WITH_OPTIONS (options) {
123-
if (crash_safe) {
124-
// Write directly to disk to avoid transport queuing during
125-
// crash
126-
sentry__run_write_envelope(options->run, envelope);
127-
sentry_envelope_free(envelope);
128-
} else {
129-
// Normal operation: use transport for HTTP transmission
130-
sentry__capture_envelope(options->transport, envelope);
131-
}
122+
if (crash_safe) {
123+
// Write directly to disk to avoid transport queuing during
124+
// crash
125+
sentry__run_write_envelope(batcher->run, envelope);
126+
sentry_envelope_free(envelope);
127+
} else if (!batcher->user_consent
128+
|| sentry__atomic_fetch(batcher->user_consent)
129+
== SENTRY_USER_CONSENT_GIVEN) {
130+
// Normal operation: use transport for HTTP transmission
131+
sentry__transport_send_envelope(batcher->transport, envelope);
132+
} else {
133+
sentry_envelope_free(envelope);
132134
}
133135
sentry_value_decref(logs);
134136
}
@@ -282,8 +284,15 @@ batcher_thread_func(void *data)
282284
}
283285

284286
void
285-
sentry__batcher_startup(sentry_batcher_t *batcher)
287+
sentry__batcher_startup(
288+
sentry_batcher_t *batcher, const sentry_options_t *options)
286289
{
290+
batcher->dsn = sentry__dsn_incref(options->dsn);
291+
batcher->transport = options->transport;
292+
batcher->run = options->run;
293+
batcher->user_consent
294+
= options->require_user_consent ? (long *)&options->user_consent : NULL;
295+
287296
// Mark thread as starting before actually spawning so thread can transition
288297
// to RUNNING. This prevents shutdown from thinking the thread was never
289298
// started if it races with the thread's initialization.
@@ -303,6 +312,8 @@ sentry__batcher_startup(sentry_batcher_t *batcher)
303312
// storage (pthread_cond_t on POSIX and CONDITION_VARIABLE on Windows)
304313
sentry__atomic_store(
305314
&batcher->thread_state, (long)SENTRY_BATCHER_THREAD_STOPPED);
315+
sentry__dsn_decref(batcher->dsn);
316+
batcher->dsn = NULL;
306317
}
307318
}
308319

@@ -338,6 +349,9 @@ sentry__batcher_shutdown_wait(sentry_batcher_t *batcher, uint64_t timeout)
338349
// Perform final flush to ensure any remaining items are sent
339350
sentry__batcher_flush(batcher, false);
340351

352+
sentry__dsn_decref(batcher->dsn);
353+
batcher->dsn = NULL;
354+
341355
sentry__thread_free(&batcher->batching_thread);
342356
}
343357

src/sentry_batcher.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
#define SENTRY_BATCHER_H_INCLUDED
33

44
#include "sentry_boot.h"
5+
#include "sentry_database.h"
56
#include "sentry_envelope.h"
67
#include "sentry_sync.h"
8+
#include "sentry_transport.h"
79

810
#ifdef SENTRY_UNITTEST
911
# define SENTRY_BATCHER_QUEUE_LENGTH 5
@@ -41,11 +43,16 @@ typedef struct {
4143
sentry_cond_t request_flush; // condition variable to schedule a flush
4244
sentry_threadid_t batching_thread; // the batching thread
4345
sentry_batch_func_t batch_func; // function to add items to envelope
46+
sentry_dsn_t *dsn;
47+
sentry_transport_t *transport;
48+
sentry_run_t *run;
49+
long *user_consent; // (atomic) NULL if consent not required
4450
} sentry_batcher_t;
4551

4652
bool sentry__batcher_flush(sentry_batcher_t *batcher, bool crash_safe);
4753
bool sentry__batcher_enqueue(sentry_batcher_t *batcher, sentry_value_t item);
48-
void sentry__batcher_startup(sentry_batcher_t *batcher);
54+
void sentry__batcher_startup(
55+
sentry_batcher_t *batcher, const sentry_options_t *options);
4956
bool sentry__batcher_shutdown_begin(sentry_batcher_t *batcher);
5057
void sentry__batcher_shutdown_wait(sentry_batcher_t *batcher, uint64_t timeout);
5158
void sentry__batcher_flush_crash_safe(sentry_batcher_t *batcher);

src/sentry_core.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,11 +299,11 @@ sentry_init(sentry_options_t *options)
299299
}
300300

301301
if (options->enable_logs) {
302-
sentry__logs_startup();
302+
sentry__logs_startup(options);
303303
}
304304

305305
if (options->enable_metrics) {
306-
sentry__metrics_startup();
306+
sentry__metrics_startup(options);
307307
}
308308

309309
sentry__mutex_unlock(&g_options_lock);

src/sentry_envelope.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,18 @@ sentry__envelope_set_header(
181181

182182
sentry_envelope_t *
183183
sentry__envelope_new(void)
184+
{
185+
sentry_dsn_t *dsn = NULL;
186+
SENTRY_WITH_OPTIONS (options) {
187+
dsn = sentry__dsn_incref(options->dsn);
188+
}
189+
sentry_envelope_t *rv = sentry__envelope_new_with_dsn(dsn);
190+
sentry__dsn_decref(dsn);
191+
return rv;
192+
}
193+
194+
sentry_envelope_t *
195+
sentry__envelope_new_with_dsn(const sentry_dsn_t *dsn)
184196
{
185197
sentry_envelope_t *rv = SENTRY_MAKE(sentry_envelope_t);
186198
if (!rv) {
@@ -193,11 +205,9 @@ sentry__envelope_new(void)
193205
rv->contents.items.item_count = 0;
194206
rv->contents.items.headers = sentry_value_new_object();
195207

196-
SENTRY_WITH_OPTIONS (options) {
197-
if (options->dsn && options->dsn->is_valid) {
198-
sentry__envelope_set_header(rv, "dsn",
199-
sentry_value_new_string(sentry_options_get_dsn(options)));
200-
}
208+
if (dsn && dsn->is_valid) {
209+
sentry__envelope_set_header(
210+
rv, "dsn", sentry_value_new_string(dsn->raw));
201211
}
202212

203213
return rv;

src/sentry_envelope.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "sentry_ratelimiter.h"
1010
#include "sentry_session.h"
1111
#include "sentry_string.h"
12+
#include "sentry_utils.h"
1213

1314
// https://develop.sentry.dev/sdk/data-model/envelopes/#size-limits
1415
#define SENTRY_MAX_ENVELOPE_SESSIONS 100
@@ -20,6 +21,11 @@ typedef struct sentry_envelope_item_s sentry_envelope_item_t;
2021
*/
2122
sentry_envelope_t *sentry__envelope_new(void);
2223

24+
/**
25+
* Create a new empty envelope with the given DSN header.
26+
*/
27+
sentry_envelope_t *sentry__envelope_new_with_dsn(const sentry_dsn_t *dsn);
28+
2329
/**
2430
* This loads a previously serialized envelope from disk.
2531
*/

src/sentry_logs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,9 +458,9 @@ sentry_log_fatal(const char *message, ...)
458458
}
459459

460460
void
461-
sentry__logs_startup(void)
461+
sentry__logs_startup(const sentry_options_t *options)
462462
{
463-
sentry__batcher_startup(&g_batcher);
463+
sentry__batcher_startup(&g_batcher, options);
464464
}
465465

466466
bool

src/sentry_logs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ log_return_value_t sentry__logs_log(
99
/**
1010
* Sets up the logs timer/flush thread
1111
*/
12-
void sentry__logs_startup(void);
12+
void sentry__logs_startup(const sentry_options_t *options);
1313

1414
/**
1515
* Begin non-blocking shutdown of the logs timer/flush thread.

src/sentry_metrics.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ sentry_metrics_distribution(
143143
}
144144

145145
void
146-
sentry__metrics_startup(void)
146+
sentry__metrics_startup(const sentry_options_t *options)
147147
{
148-
sentry__batcher_startup(&g_batcher);
148+
sentry__batcher_startup(&g_batcher, options);
149149
}
150150

151151
bool

src/sentry_metrics.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
/**
77
* Sets up the metrics timer/flush thread
88
*/
9-
void sentry__metrics_startup(void);
9+
void sentry__metrics_startup(const sentry_options_t *options);
1010

1111
/**
1212
* Begin non-blocking shutdown of the metrics timer/flush thread.

0 commit comments

Comments
 (0)