Skip to content

Commit

Permalink
fix: Lock global options whenever they are accessed
Browse files Browse the repository at this point in the history
This also changes the DSN to a heap allocated, refcounted type since
the http worker thread needs to hold onto its own reference.
  • Loading branch information
Swatinem committed Jul 22, 2020
1 parent b1debcb commit 7e81099
Show file tree
Hide file tree
Showing 23 changed files with 445 additions and 383 deletions.
2 changes: 1 addition & 1 deletion .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ IndentPPDirectives: AfterHash
ColumnLimit: 80
AlwaysBreakAfterDefinitionReturnType: All
PointerAlignment: Right
ForEachMacros: ['SENTRY_WITH_SCOPE', 'SENTRY_WITH_SCOPE_MUT']
ForEachMacros: ['SENTRY_WITH_SCOPE', 'SENTRY_WITH_SCOPE_MUT', 'SENTRY_WITH_SCOPE_MUT_NO_FLUSH', 'SENTRY_WITH_OPTIONS']
4 changes: 3 additions & 1 deletion include/sentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -906,9 +906,11 @@ SENTRY_API int sentry_shutdown(void);
SENTRY_EXPERIMENTAL_API void sentry_clear_modulecache(void);

/**
* Returns the client options.
* Returns the currently active client options.
*
* This might return NULL if sentry is not yet initialized.
* The returned options pointer is not synchronized, and calling
* `sentry_shutdown` concurrently can lead to use-after-free situations.
*/
SENTRY_API const sentry_options_t *sentry_get_options(void);

Expand Down
84 changes: 49 additions & 35 deletions src/backends/sentry_backend_breakpad.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ sentry__breakpad_transport_free(void *_state)
sentry_free(state);
}

static void
sentry__enforce_breakpad_transport(
static sentry_transport_t *
sentry__breakpad_transport_new(
const sentry_options_t *options, sentry_path_t *dump_path)
{
breakpad_transport_state_t *state = SENTRY_MAKE(breakpad_transport_state_t);
if (!state) {
sentry__path_free(dump_path);
return;
return NULL;
}
state->run = options->run;
state->dump_path = dump_path;
Expand All @@ -98,12 +98,22 @@ sentry__enforce_breakpad_transport(
= sentry_transport_new(sentry__breakpad_backend_send_envelope);
if (!transport) {
sentry__breakpad_transport_free(state);
return;
return NULL;
}
sentry_transport_set_state(transport, state);
sentry_transport_set_free_func(transport, sentry__breakpad_transport_free);
return transport;
}

((sentry_options_t *)options)->transport = transport;
static sentry_transport_t *
sentry__swap_breakpad_transport(
sentry_options_t *options, sentry_path_t *dump_path)
{
sentry_transport_t *breakpad_transport
= sentry__breakpad_transport_new(options, dump_path);
sentry_transport_t *transport = options->transport;
options->transport = breakpad_transport;
return transport;
}

#ifdef SENTRY_PLATFORM_WINDOWS
Expand All @@ -119,14 +129,13 @@ sentry__breakpad_backend_callback(
void *UNUSED(context), bool succeeded)
#endif
{
SENTRY_DEBUG("entering breakpad minidump callback");

#ifndef SENTRY_PLATFORM_WINDOWS
sentry__page_allocator_enable();
sentry__enter_signal_handler();
#endif

const sentry_options_t *options = sentry_get_options();
sentry__write_crash_marker(options);

sentry_path_t *dump_path = nullptr;
#ifdef SENTRY_PLATFORM_WINDOWS
sentry_path_t *tmp_path = sentry__path_new(breakpad_dump_path);
Expand All @@ -139,36 +148,41 @@ sentry__breakpad_backend_callback(
dump_path = sentry__path_new(descriptor.path());
#endif

// since we can’t use HTTP in crash handlers, we will swap out the
// transport here to one that serializes the envelope to disk
sentry_transport_t *transport_original = options->transport;
sentry__enforce_disk_transport();

// Ending the session will send an envelope, which we *don’t* want to route
// through the special transport. It will be dumped to disk with the rest of
// the send queue.
sentry__end_current_session_with_status(SENTRY_SESSION_STATUS_CRASHED);

// almost identical to enforcing the disk transport, the breakpad
// transport will serialize the envelope to disk, but will attach the
// captured minidump as an additional attachment
sentry_transport_t *transport_disk = options->transport;
sentry__enforce_breakpad_transport(options, dump_path);

// after the transport is set up, we will capture an event, which will
// create an envelope with all the scope, attachments, etc.
sentry_value_t event = sentry_value_new_event();
sentry_capture_event(event);

// after capturing the crash event, try to dump all the in-flight data of
// the previous transports
if (transport_disk) {
SENTRY_WITH_OPTIONS (options) {
sentry__write_crash_marker(options);

// since we can’t use HTTP in crash handlers, we will swap out the
// transport here to one that serializes the envelope to disk
sentry_transport_t *transport_original
= sentry__swap_disk_transport(options);

// Ending the session will send an envelope, which we *don’t* want to
// route through the special transport. It will be dumped to disk with
// the rest of the send queue.
sentry__end_current_session_with_status(SENTRY_SESSION_STATUS_CRASHED);

// almost identical to enforcing the disk transport, the breakpad
// transport will serialize the envelope to disk, but will attach
// the captured minidump as an additional attachment
sentry_transport_t *transport_disk
= sentry__swap_breakpad_transport(options, dump_path);

// after the transport is set up, we will capture an event, which
// will create an envelope with all the scope, attachments, etc.
sentry_value_t event = sentry_value_new_event();
sentry_capture_event(event);

// after capturing the crash event, try to dump all the in-flight
// data of the previous transports
sentry__transport_dump_queue(options->transport, options->run);
sentry__transport_dump_queue(transport_disk, options->run);
}
if (transport_original) {
sentry__transport_dump_queue(transport_original, options->run);
// and restore the old transport
sentry_transport_free(options->transport);
sentry_transport_free(transport_disk);
options->transport = transport_original;
SENTRY_DEBUG("crash has been captured");
}
SENTRY_DEBUG("crash has been captured");

#ifndef SENTRY_PLATFORM_WINDOWS
sentry__leave_signal_handler();
Expand Down
3 changes: 2 additions & 1 deletion src/backends/sentry_backend_crashpad.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ sentry__crashpad_backend_startup(
data->db = crashpad::CrashReportDatabase::Initialize(database).release();

crashpad::CrashpadClient client;
char *minidump_url = sentry__dsn_get_minidump_url(&options->dsn);
char *minidump_url = sentry__dsn_get_minidump_url(options->dsn);
SENTRY_TRACEF("using minidump url \"%s\"", minidump_url);
std::string url = minidump_url ? std::string(minidump_url) : std::string();
sentry_free(minidump_url);
bool success = client.StartHandler(handler, database, database, url,
Expand Down
33 changes: 17 additions & 16 deletions src/backends/sentry_backend_inproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,8 @@ make_signal_event(
static void
handle_ucontext(const sentry_ucontext_t *uctx)
{
SENTRY_DEBUG("entering signal handler");

const struct signal_slot *sig_slot = NULL;
for (int i = 0; i < SIGNAL_COUNT; ++i) {
#ifdef SENTRY_PLATFORM_UNIX
Expand All @@ -257,26 +259,25 @@ handle_ucontext(const sentry_ucontext_t *uctx)
sentry__enter_signal_handler();
#endif

const sentry_options_t *opts = sentry_get_options();
sentry__write_crash_marker(opts);
SENTRY_WITH_OPTIONS (options) {
sentry__write_crash_marker(options);

// since we can’t use HTTP in signal handlers, we will swap out the
// transport here to one that serializes the envelope to disk
sentry_transport_t *transport = opts->transport;
sentry__enforce_disk_transport();
// since we can’t use HTTP in signal handlers, we will swap out the
// transport here to one that serializes the envelope to disk
sentry_transport_t *transport = sentry__swap_disk_transport(options);

// now create an capture an event. Note that this assumes the transport
// only dumps to disk at the moment.
SENTRY_DEBUG("capturing event from signal");
sentry__end_current_session_with_status(SENTRY_SESSION_STATUS_CRASHED);
sentry_capture_event(make_signal_event(sig_slot, uctx));
// now create an capture an event.
sentry__end_current_session_with_status(SENTRY_SESSION_STATUS_CRASHED);
sentry_capture_event(make_signal_event(sig_slot, uctx));

// after capturing the crash event, try to dump all the in-flight data of
// the previous transport
if (transport) {
sentry__transport_dump_queue(transport, opts->run);
// after capturing the crash event, dump all the envelopes to disk
sentry__transport_dump_queue(options->transport, options->run);
sentry__transport_dump_queue(transport, options->run);
// and restore the old transport
sentry_transport_free(options->transport);
options->transport = transport;
SENTRY_DEBUG("crash has been captured");
}
SENTRY_DEBUG("crash has been captured");

#ifdef SENTRY_PLATFORM_UNIX
// reset signal handlers and invoke the original ones. This will then tear
Expand Down
Loading

0 comments on commit 7e81099

Please sign in to comment.