Skip to content

Commit

Permalink
Merge branch 'master' into feat/socks5_proxy_joshua
Browse files Browse the repository at this point in the history
# Conflicts:
#	CHANGELOG.md
  • Loading branch information
JoshuaMoelans committed Nov 18, 2024
2 parents 7233e44 + 7c1d428 commit abca3c5
Show file tree
Hide file tree
Showing 14 changed files with 330 additions and 41 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
**Features**:

- Provide version information for non-static Windows binaries. ([#1076](https://github.com/getsentry/sentry-native/pull/1076), [crashpad#110](https://github.com/getsentry/crashpad/pull/110))
- Add an alternative handler strategy to `inproc` to support `.NET` on Linux and `Mono` on Android (specifically, [.NET MAUI](https://github.com/dotnet/android/issues/9055#issuecomment-2261347912)). ([#1027](https://github.com/getsentry/sentry-native/pull/1027))
- Add SOCKS5 proxy support for macOS and Linux. ([#1063](https://github.com/getsentry/sentry-native/pull/1063))

**Fixes**:

- Correct the timeout specified for the upload-task awaiting `dispatch_semaphore_wait()` when using an HTTP-proxy on macOS. ([#1077](https://github.com/getsentry/sentry-native/pull/1077), [crashpad#111](https://github.com/getsentry/crashpad/pull/111))
- Emit `transaction.data` inside `context.trace.data`. ([#1075](https://github.com/getsentry/sentry-native/pull/1075))

**Thank you**:

Expand Down
38 changes: 38 additions & 0 deletions include/sentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,14 @@ typedef enum {
SENTRY_USER_CONSENT_REVOKED = 0,
} sentry_user_consent_t;

/**
* The crash handler strategy.
*/
typedef enum {
SENTRY_HANDLER_STRATEGY_DEFAULT = 0,
SENTRY_HANDLER_STRATEGY_CHAIN_AT_START = 1,
} sentry_handler_strategy_t;

/**
* Creates a new options struct.
* Can be freed with `sentry_options_free`.
Expand Down Expand Up @@ -1511,6 +1519,36 @@ SENTRY_EXPERIMENTAL_API void sentry_options_set_traces_sample_rate(
SENTRY_EXPERIMENTAL_API double sentry_options_get_traces_sample_rate(
sentry_options_t *opts);

#ifdef SENTRY_PLATFORM_LINUX

/**
* Returns the currently set strategy for the handler.
*
* This option does only work for the `inproc` backend on `Linux` and `Android`.
*
* The main use-case is when the Native SDK is used in the context of the
* CLR/Mono runtimes which convert some POSIX signals into managed-code
* exceptions and discontinue the signal chain.
*
* If this happens and we invoke the previous handler at the end (i.e., after
* our handler processed the signal, which is the default strategy) we will end
* up sending a native crash in addition to the managed code exception (which
* will either generate another crash-event if uncaught or could be handled in
* the managed code and neither terminate the application nor create a crash
* event). To correctly process the signals of CLR/Mono applications, we must
* invoke the runtime handler at the start of our handler.
*/
SENTRY_EXPERIMENTAL_API sentry_handler_strategy_t
sentry_options_get_handler_strategy(const sentry_options_t *opts);

/**
* Sets the handler strategy.
*/
SENTRY_EXPERIMENTAL_API void sentry_options_set_handler_strategy(
sentry_options_t *opts, sentry_handler_strategy_t handler_strategy);

#endif // SENTRY_PLATFORM_LINUX

/* -- Session APIs -- */

typedef enum {
Expand Down
73 changes: 40 additions & 33 deletions src/backends/sentry_backend_inproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,6 @@
#include "transports/sentry_disk_transport.h"
#include <string.h>

/**
* Android's bionic libc seems to allocate alternate signal handler stacks for
* every thread and also references them from their internal maintenance
* structs.
*
* The way we currently set up our sigaltstack seems to interfere with this
* setup and causes crashes whenever an ART signal handler touches the thread
* that called `sentry_init()`.
*
* In addition to this problem, it also means there is no need for our own
* sigaltstack on Android since our signal handler will always be running on
* an alternate stack managed by bionic.
*
* Note: In bionic the sigaltstacks for 32-bit devices have a size of 16KiB and
* on 64-bit devices they have 32KiB. The size of our own was set to 64KiB
* independent of the device. If this is a problem, we need figure out
* together with Google if there is a way in which our configs can coexist.
*
* Both breakpad and crashpad are way more defensive in the setup of their
* signal stacks and take existing stacks into account (or reuse them).
*/
#define SIGNAL_DEF(Sig, Desc) { Sig, #Sig, Desc }

#define MAX_FRAMES 128
Expand Down Expand Up @@ -108,8 +87,8 @@ startup_inproc_backend(

// set up an alternate signal stack if noone defined one before
stack_t old_sig_stack;
if (sigaltstack(NULL, &old_sig_stack) == -1 || old_sig_stack.ss_sp == NULL
|| old_sig_stack.ss_size == 0) {
int ret = sigaltstack(NULL, &old_sig_stack);
if (ret == 0 && old_sig_stack.ss_flags == SS_DISABLE) {
SENTRY_TRACEF("installing signal stack (size: %d)", SIGNAL_STACK_SIZE);
g_signal_stack.ss_sp = sentry_malloc(SIGNAL_STACK_SIZE);
if (!g_signal_stack.ss_sp) {
Expand All @@ -118,9 +97,11 @@ startup_inproc_backend(
g_signal_stack.ss_size = SIGNAL_STACK_SIZE;
g_signal_stack.ss_flags = 0;
sigaltstack(&g_signal_stack, 0);
} else {
SENTRY_TRACEF(
"using existing signal stack (size: %d)", old_sig_stack.ss_size);
} else if (ret == 0) {
SENTRY_TRACEF("using existing signal stack (size: %d, flags: %d)",
old_sig_stack.ss_size, old_sig_stack.ss_flags);
} else if (ret == -1) {
SENTRY_WARNF("Failed to query signal stack size: %s", strerror(errno));
}

// install our own signal handler
Expand Down Expand Up @@ -554,21 +535,47 @@ handle_ucontext(const sentry_ucontext_t *uctx)
}

#ifdef SENTRY_PLATFORM_UNIX
// give us an allocator we can use safely in signals before we tear down.
sentry__page_allocator_enable();

// inform the sentry_sync system that we're in a signal handler. This will
// make mutexes spin on a spinlock instead as it's no longer safe to use a
// pthread mutex.
sentry__enter_signal_handler();
#endif

sentry_value_t event = make_signal_event(sig_slot, uctx);

SENTRY_WITH_OPTIONS (options) {
sentry__write_crash_marker(options);
#ifdef SENTRY_PLATFORM_LINUX
// On Linux (and thus Android) CLR/Mono converts signals provoked by
// AOT/JIT-generated native code into managed code exceptions. In these
// cases, we shouldn't react to the signal at all and let their handler
// discontinue the signal chain by invoking the runtime handler before
// we process the signal.
if (sentry_options_get_handler_strategy(options)
== SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) {
SENTRY_TRACE("defer to runtime signal handler at start");
// there is a good chance that we won't return from the previous
// handler and that would mean we couldn't enter this handler with
// the next signal coming in if we didn't "leave" here.
sentry__leave_signal_handler();

// invoke the previous handler (typically the CLR/Mono
// signal-to-managed-exception handler)
invoke_signal_handler(
uctx->signum, uctx->siginfo, (void *)uctx->user_context);

// let's re-enter because it means this was an actual native crash
sentry__enter_signal_handler();
SENTRY_TRACE(
"return from runtime signal handler, we handle the signal");
}
#endif

#ifdef SENTRY_PLATFORM_UNIX
// use a signal-safe allocator before we tear down.
sentry__page_allocator_enable();
#endif

sentry_value_t event = make_signal_event(sig_slot, uctx);
bool should_handle = true;
sentry__write_crash_marker(options);

if (options->on_crash_func) {
SENTRY_TRACE("invoking `on_crash` hook");
Expand All @@ -580,7 +587,7 @@ handle_ucontext(const sentry_ucontext_t *uctx)
sentry_envelope_t *envelope = sentry__prepare_event(
options, event, NULL, !options->on_crash_func);
// TODO(tracing): Revisit when investigating transaction flushing
// during hard crashes.
// during hard crashes.

sentry_session_t *session = sentry__end_current_session_with_status(
SENTRY_SESSION_STATUS_CRASHED);
Expand Down
4 changes: 4 additions & 0 deletions src/sentry_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,9 @@ sentry_transaction_finish(sentry_transaction_t *opaque_tx)
sentry_value_t trace_context
= sentry__value_get_trace_context(opaque_tx->inner);
sentry_value_t contexts = sentry_value_new_object();
sentry_value_set_by_key(
trace_context, "data", sentry_value_get_by_key(tx, "data"));
sentry_value_incref(sentry_value_get_by_key(tx, "data"));
sentry_value_set_by_key(contexts, "trace", trace_context);
sentry_value_set_by_key(tx, "contexts", contexts);

Expand All @@ -944,6 +947,7 @@ sentry_transaction_finish(sentry_transaction_t *opaque_tx)
sentry_value_remove_by_key(tx, "op");
sentry_value_remove_by_key(tx, "description");
sentry_value_remove_by_key(tx, "status");
sentry_value_remove_by_key(tx, "data");

sentry__transaction_decref(opaque_tx);

Expand Down
18 changes: 18 additions & 0 deletions src/sentry_options.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ sentry_options_new(void)
opts->shutdown_timeout = SENTRY_DEFAULT_SHUTDOWN_TIMEOUT;
opts->traces_sample_rate = 0.0;
opts->max_spans = 0;
opts->handler_strategy = SENTRY_HANDLER_STRATEGY_DEFAULT;

return opts;
}
Expand Down Expand Up @@ -613,3 +614,20 @@ sentry_options_set_backend(sentry_options_t *opts, sentry_backend_t *backend)
sentry__backend_free(opts->backend);
opts->backend = backend;
}

#ifdef SENTRY_PLATFORM_LINUX

sentry_handler_strategy_t
sentry_options_get_handler_strategy(const sentry_options_t *opts)
{
return opts->handler_strategy;
}

void
sentry_options_set_handler_strategy(
sentry_options_t *opts, sentry_handler_strategy_t handler_strategy)
{
opts->handler_strategy = handler_strategy;
}

#endif // SENTRY_PLATFORM_LINUX
1 change: 1 addition & 0 deletions src/sentry_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ typedef struct sentry_options_s {
long user_consent;
long refcount;
uint64_t shutdown_timeout;
sentry_handler_strategy_t handler_strategy;
} sentry_options_t;

/**
Expand Down
12 changes: 10 additions & 2 deletions src/sentry_scope.c
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,20 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope,

// prep contexts sourced from scope; data about transaction on scope needs
// to be extracted and inserted
sentry_value_t scope_trace = sentry__value_get_trace_context(
sentry__get_span_or_transaction(scope));
sentry_value_t scoped_txn_or_span = sentry__get_span_or_transaction(scope);
sentry_value_t scope_trace
= sentry__value_get_trace_context(scoped_txn_or_span);
if (!sentry_value_is_null(scope_trace)) {
if (sentry_value_is_null(contexts)) {
contexts = sentry_value_new_object();
}
sentry_value_t scoped_txn_or_span_data
= sentry_value_get_by_key(scoped_txn_or_span, "data");
if (!sentry_value_is_null(scoped_txn_or_span_data)) {
sentry_value_incref(scoped_txn_or_span_data);
sentry_value_set_by_key(
scope_trace, "data", scoped_txn_or_span_data);
}
sentry_value_set_by_key(contexts, "trace", scope_trace);
}

Expand Down
2 changes: 1 addition & 1 deletion src/sentry_tracing.c
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ sentry_transaction_set_data(
}
}

static const char txn_data_key[] = "extra";
static const char txn_data_key[] = "data";
static const size_t txn_data_key_len = sizeof(txn_data_key) - 1;

void
Expand Down
64 changes: 64 additions & 0 deletions tests/fixtures/dotnet_signal/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
namespace dotnet_signal;

using System;
using System.Runtime.InteropServices;

class Program
{
[DllImport("crash", EntryPoint = "native_crash")]
static extern void native_crash();

[DllImport("crash", EntryPoint = "enable_sigaltstack")]
static extern void enable_sigaltstack();

[DllImport("sentry", EntryPoint = "sentry_options_new")]
static extern IntPtr sentry_options_new();

[DllImport("sentry", EntryPoint = "sentry_options_set_handler_strategy")]
static extern IntPtr sentry_options_set_handler_strategy(IntPtr options, int strategy);

[DllImport("sentry", EntryPoint = "sentry_options_set_debug")]
static extern IntPtr sentry_options_set_debug(IntPtr options, int debug);

[DllImport("sentry", EntryPoint = "sentry_init")]
static extern int sentry_init(IntPtr options);

static void Main(string[] args)
{
var githubActions = Environment.GetEnvironmentVariable("GITHUB_ACTIONS") ?? string.Empty;
if (githubActions == "true") {
// Set up our own `sigaltstack` for this thread if we're running on GHA because of a failure to run any
// signal handler after the initial setup. This behavior is locally non-reproducible and likely runner-related.
// I ran this against .net7/8/9 on at least 10 different Linux setups, and it worked on all, but on GHA
// it only works if we __don't__ accept the already installed `sigaltstack`.
enable_sigaltstack();
}

// setup minimal sentry-native
var options = sentry_options_new();
sentry_options_set_handler_strategy(options, 1);
sentry_options_set_debug(options, 1);
sentry_init(options);

var doNativeCrash = args is ["native-crash"];
if (doNativeCrash)
{
native_crash();
}
else
{
try
{
Console.WriteLine("dereference a NULL object from managed code");
var s = default(string);
var c = s.Length;
}
catch (NullReferenceException exception)
{
Console.WriteLine("dereference another NULL object from managed code");
var s = default(string);
var c = s.Length;
}
}
}
}
19 changes: 19 additions & 0 deletions tests/fixtures/dotnet_signal/crash.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#include <signal.h>
#include <stdlib.h>
void native_crash(void)
{
*(int *)10 = 100;
}

void enable_sigaltstack(void)
{
const size_t signal_stack_size = 16384;
stack_t signal_stack;
signal_stack.ss_sp = malloc(signal_stack_size);
if (!signal_stack.ss_sp) {
return;
}
signal_stack.ss_size = signal_stack_size;
signal_stack.ss_flags = 0;
sigaltstack(&signal_stack, 0);
}
8 changes: 8 additions & 0 deletions tests/fixtures/dotnet_signal/test_dotnet.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>
</Project>
Loading

0 comments on commit abca3c5

Please sign in to comment.