diff --git a/src/sentry_batcher.c b/src/sentry_batcher.c index 89116e63f..8bf44ebd8 100644 --- a/src/sentry_batcher.c +++ b/src/sentry_batcher.c @@ -224,9 +224,7 @@ sentry__batcher_flush(sentry_batcher_t *batcher, bool crash_safe) // crash sentry__run_write_envelope(batcher->run, envelope); sentry_envelope_free(envelope); - } else if (!batcher->user_consent - || sentry__atomic_fetch(batcher->user_consent) - == SENTRY_USER_CONSENT_GIVEN) { + } else if (!sentry__run_should_skip_upload(batcher->run)) { // Normal operation: use transport for HTTP transmission sentry__transport_send_envelope(batcher->transport, envelope); } else { @@ -374,12 +372,10 @@ sentry__batcher_startup( { // dsn is incref'd because release() decref's it and may outlive options. batcher->dsn = sentry__dsn_incref(options->dsn); - // transport, run, and user_consent are non-owning refs, safe because they + // transport and run are non-owning refs, safe because they // are only accessed in flush() which is bound by the options lifetime. batcher->transport = options->transport; batcher->run = options->run; - batcher->user_consent - = options->require_user_consent ? (long *)&options->user_consent : NULL; // Mark thread as starting before actually spawning so thread can transition // to RUNNING. This prevents shutdown from thinking the thread was never diff --git a/src/sentry_batcher.h b/src/sentry_batcher.h index 29563bf74..53a0e4bb0 100644 --- a/src/sentry_batcher.h +++ b/src/sentry_batcher.h @@ -49,7 +49,6 @@ typedef struct { sentry_dsn_t *dsn; sentry_transport_t *transport; sentry_run_t *run; - long *user_consent; // (atomic) NULL if consent not required } sentry_batcher_t; typedef struct { diff --git a/src/sentry_core.c b/src/sentry_core.c index 6ab61a810..4b617c315 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -71,35 +71,12 @@ sentry__options_unlock(void) sentry__mutex_unlock(&g_options_lock); } -static void -load_user_consent(sentry_options_t *opts) -{ - sentry_path_t *consent_path - = sentry__path_join_str(opts->database_path, "user-consent"); - char *contents = sentry__path_read_to_buffer(consent_path, NULL); - sentry__path_free(consent_path); - switch (contents ? contents[0] : 0) { - case '1': - opts->user_consent = SENTRY_USER_CONSENT_GIVEN; - break; - case '0': - opts->user_consent = SENTRY_USER_CONSENT_REVOKED; - break; - default: - opts->user_consent = SENTRY_USER_CONSENT_UNKNOWN; - break; - } - sentry_free(contents); -} - bool sentry__should_skip_upload(void) { bool skip = true; SENTRY_WITH_OPTIONS (options) { - skip = options->require_user_consent - && sentry__atomic_fetch((long *)&options->user_consent) - != SENTRY_USER_CONSENT_GIVEN; + skip = sentry__run_should_skip_upload(options->run); } return skip; } @@ -207,8 +184,9 @@ sentry_init(sentry_options_t *options) SENTRY_WARN("failed to initialize run directory"); goto fail; } + options->run->require_user_consent = options->require_user_consent; - load_user_consent(options); + sentry__run_load_user_consent(options->run, options->database_path); if (!options->dsn || !options->dsn->is_valid) { const char *raw_dsn = sentry_options_get_dsn(options); @@ -437,7 +415,7 @@ static void set_user_consent(sentry_user_consent_t new_val) { SENTRY_WITH_OPTIONS (options) { - if (sentry__atomic_store((long *)&options->user_consent, new_val) + if (sentry__atomic_store(&options->run->user_consent, new_val) != new_val) { if (options->backend && options->backend->user_consent_changed_func) { @@ -486,7 +464,7 @@ sentry_user_consent_get(void) sentry_user_consent_t rv = SENTRY_USER_CONSENT_UNKNOWN; SENTRY_WITH_OPTIONS (options) { rv = (sentry_user_consent_t)(int)sentry__atomic_fetch( - (long *)&options->user_consent); + &options->run->user_consent); } return rv; } diff --git a/src/sentry_core.h b/src/sentry_core.h index d78688b41..0c0b03e87 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -34,6 +34,10 @@ /** * This function will check the user consent, and return `true` if uploads * should *not* be sent to the sentry server, and be discarded instead. + * + * Note: This function acquires the options lock internally. Use + * `sentry__run_should_skip_upload` from worker threads that may run while + * the options are locked during SDK shutdown. */ bool sentry__should_skip_upload(void); diff --git a/src/sentry_database.c b/src/sentry_database.c index fd4f2f14c..fe04d0440 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -74,6 +74,8 @@ sentry__run_new(const sentry_path_t *database_path) } run->refcount = 1; + run->require_user_consent = 0; + run->user_consent = SENTRY_USER_CONSENT_UNKNOWN; run->uuid = uuid; run->run_path = run_path; run->session_path = session_path; @@ -96,6 +98,36 @@ sentry__run_new(const sentry_path_t *database_path) return NULL; } +bool +sentry__run_should_skip_upload(sentry_run_t *run) +{ + return sentry__atomic_fetch(&run->require_user_consent) + && (sentry__atomic_fetch(&run->user_consent) + != SENTRY_USER_CONSENT_GIVEN); +} + +void +sentry__run_load_user_consent( + sentry_run_t *run, const sentry_path_t *database_path) +{ + sentry_path_t *consent_path + = sentry__path_join_str(database_path, "user-consent"); + char *contents = sentry__path_read_to_buffer(consent_path, NULL); + sentry__path_free(consent_path); + switch (contents ? contents[0] : 0) { + case '1': + sentry__atomic_store(&run->user_consent, SENTRY_USER_CONSENT_GIVEN); + break; + case '0': + sentry__atomic_store(&run->user_consent, SENTRY_USER_CONSENT_REVOKED); + break; + default: + sentry__atomic_store(&run->user_consent, SENTRY_USER_CONSENT_UNKNOWN); + break; + } + sentry_free(contents); +} + sentry_run_t * sentry__run_incref(sentry_run_t *run) { diff --git a/src/sentry_database.h b/src/sentry_database.h index 33e6735ce..e42cd4289 100644 --- a/src/sentry_database.h +++ b/src/sentry_database.h @@ -14,8 +14,25 @@ typedef struct sentry_run_s { sentry_path_t *cache_path; sentry_filelock_t *lock; long refcount; + long require_user_consent; // (atomic) bool + long user_consent; // (atomic) sentry_user_consent_t } sentry_run_t; +/** + * This function will check the user consent, and return `true` if uploads + * should *not* be sent to the sentry server, and be discarded instead. + * + * This is a lock-free variant of `sentry__should_skip_upload`, safe to call + * from worker threads while the options are locked during SDK shutdown. + */ +bool sentry__run_should_skip_upload(sentry_run_t *run); + +/** + * Loads the persisted user consent (`/user-consent`) into the run. + */ +void sentry__run_load_user_consent( + sentry_run_t *run, const sentry_path_t *database_path); + /** * This creates a new application run including its associated directory and * lockfile: diff --git a/src/sentry_options.c b/src/sentry_options.c index 1f0dedbe2..45aa9bbc6 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -45,7 +45,6 @@ sentry_options_new(void) } sentry_options_set_sdk_name(opts, SENTRY_SDK_NAME); opts->max_breadcrumbs = SENTRY_BREADCRUMBS_MAX; - opts->user_consent = SENTRY_USER_CONSENT_UNKNOWN; opts->auto_session_tracking = true; opts->system_crash_reporter_enabled = false; opts->attach_screenshot = false; diff --git a/src/sentry_options.h b/src/sentry_options.h index 0ca46a3ca..86ee949c2 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -88,7 +88,6 @@ struct sentry_options_s { struct sentry_backend_s *backend; sentry_session_t *session; - long user_consent; long refcount; uint64_t shutdown_timeout; sentry_handler_strategy_t handler_strategy;