Skip to content
1 change: 1 addition & 0 deletions samples/Sentry.Samples.Maui/MauiProgram.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public static MauiApp CreateMauiApp()
// but only if tracing is enabled. Here we capture all traces (in a production app you'd probably only
// capture a certain percentage)
options.TracesSampleRate = 1.0F;
options.EnableNavigationTransactions = true;

// Automatically create traces for async relay commands in the MVVM Community Toolkit
options.AddCommunityToolkitIntegration();
Expand Down
4 changes: 4 additions & 0 deletions src/Sentry.Maui/BindableSentryMauiOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ internal class BindableSentryMauiOptions : BindableSentryLoggingOptions
public bool? IncludeBackgroundingStateInBreadcrumbs { get; set; }
public bool? CreateElementEventsBreadcrumbs { get; set; } = false;
public bool? AttachScreenshot { get; set; }
public bool? EnableNavigationTransactions { get; set; }
public TimeSpan? NavigationTransactionIdleTimeout { get; set; }

public void ApplyTo(SentryMauiOptions options)
{
Expand All @@ -19,5 +21,7 @@ public void ApplyTo(SentryMauiOptions options)
options.IncludeBackgroundingStateInBreadcrumbs = IncludeBackgroundingStateInBreadcrumbs ?? options.IncludeBackgroundingStateInBreadcrumbs;
options.CreateElementEventsBreadcrumbs = CreateElementEventsBreadcrumbs ?? options.CreateElementEventsBreadcrumbs;
options.AttachScreenshot = AttachScreenshot ?? options.AttachScreenshot;
options.EnableNavigationTransactions = EnableNavigationTransactions ?? options.EnableNavigationTransactions;
options.NavigationTransactionIdleTimeout = NavigationTransactionIdleTimeout ?? options.NavigationTransactionIdleTimeout;
}
}
97 changes: 92 additions & 5 deletions src/Sentry.Maui/Internal/MauiEventsBinder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.Extensions.Options;
using Sentry.Internal;

namespace Sentry.Maui.Internal;

Expand All @@ -13,6 +14,10 @@
private readonly SentryMauiOptions _options;
internal readonly IEnumerable<IMauiElementEventBinder> _elementEventBinders;

// Tracks the active auto-finishing navigation transaction so we can explicitly finish it early
// (e.g. when the next navigation begins) before the idle timeout would fire.
private ITransactionTracer? _currentTransaction;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Race condition: _currentTransaction field accessed without synchronization in singleton service

The _currentTransaction field is added to a singleton MauiEventsBinder and accessed from multiple event handlers (OnShellOnNavigating, OnApplicationOnModalPushed, OnApplicationOnModalPopped, OnWindowOnStopped) without thread synchronization. These handlers can fire concurrently from different navigation events. The StartNavigationTransaction method has a TOCTOU race where it checks _currentTransaction is { IsFinished: false } and then conditionally finishes or resets it - another thread could modify _currentTransaction between the check and the subsequent operations.

Verification

Read the full MauiEventsBinder.cs to trace all usages of _currentTransaction. Confirmed it's registered as singleton via SentryMauiAppBuilderExtensions.cs line 79. Found multiple event handlers modifying this field: StartNavigationTransaction (lines 327-366), OnApplicationOnModalPopped (lines 388-392), OnWindowOnStopped (lines 407-411). The StartNavigationTransaction method has check-then-act logic without locks (lines 330-334, 337, 364).

Identified by Warden find-bugs · HKG-HRW


// https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/#breadcrumb-types
// https://github.com/getsentry/sentry/blob/master/static/app/types/breadcrumbs.tsx
internal const string NavigationType = "navigation";
Expand Down Expand Up @@ -319,16 +324,76 @@
}
}

private ITransactionTracer? StartNavigationTransaction(string name)
{
// If there's already a transaction on the scope that we didn't create, it was put there
// manually by the user — don't override it.
_hub.ConfigureScope(scope =>
{
if (scope.Transaction is { } existing && !ReferenceEquals(existing, _currentTransaction))
{
_manualTransactionOnScope = true;
}
});
if (_manualTransactionOnScope)
{
return null;
}

Check warning on line 341 in src/Sentry.Maui/Internal/MauiEventsBinder.cs

View check run for this annotation

@sentry/warden / warden: find-bugs

_manualTransactionOnScope flag is never cleared, permanently disabling auto navigation transactions

The comment on lines 368-369 states '_manualTransactionOnScope' should be 'cleared on the next navigation so we re-evaluate', but the flag is never actually reset to false after being set to true. Once a user-created transaction is detected on the scope, the SDK will permanently stop creating automatic navigation transactions for the lifetime of the MauiEventsBinder instance, even after the user's transaction finishes.

Check warning on line 341 in src/Sentry.Maui/Internal/MauiEventsBinder.cs

View check run for this annotation

@sentry/warden / warden: code-review

_manualTransactionOnScope is never reset, permanently disabling navigation transactions after first manual transaction

The comment on line 368-369 states that `_manualTransactionOnScope` should be 'cleared on the next navigation so we re-evaluate', but the code never sets this flag back to `false`. Once a user's manual transaction is detected, `_manualTransactionOnScope` remains `true` permanently for the singleton `MauiEventsBinder`, causing all subsequent navigation events to skip creating auto-transactions even after the user's transaction has finished. This contradicts the stated design intent.

// Same destination as the current transaction — reset the idle timeout instead of
// creating a new transaction.
if (_currentTransaction is { IsFinished: false } current && current.Name == name)
{
current.ResetIdleTimeout();
return current;
}

// Finish any previous SDK-owned navigation transaction before starting a new one.
_currentTransaction?.Finish(SpanStatus.Ok);

var context = new TransactionContext(name, "ui.load")
{
NameSource = TransactionNameSource.Route
};

var transaction = _hub is IHubInternal internalHub
? internalHub.StartTransaction(context, _options.NavigationTransactionIdleTimeout)
: _hub.StartTransaction(context);

_hub.ConfigureScope(static (scope, t) => scope.Transaction = t, transaction);
_currentTransaction = transaction;
return transaction;
}

// Set to true when we detect a user-created transaction on the scope; cleared on the next
// navigation so we re-evaluate (the user's transaction may have finished by then).
private bool _manualTransactionOnScope;

// Application Events

private void OnApplicationOnPageAppearing(object? sender, Page page) =>
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.PageAppearing), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, page, nameof(Page)));
private void OnApplicationOnPageDisappearing(object? sender, Page page) =>
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.PageDisappearing), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, page, nameof(Page)));
private void OnApplicationOnModalPushed(object? sender, ModalPushedEventArgs e) =>

private void OnApplicationOnModalPushed(object? sender, ModalPushedEventArgs e)
{
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.ModalPushed), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, e.Modal, nameof(e.Modal)));
private void OnApplicationOnModalPopped(object? sender, ModalPoppedEventArgs e) =>
if (_options.EnableNavigationTransactions)
{
StartNavigationTransaction(e.Modal.GetType().Name);
}
}

private void OnApplicationOnModalPopped(object? sender, ModalPoppedEventArgs e)
{
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.ModalPopped), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, e.Modal, nameof(e.Modal)));
if (_options.EnableNavigationTransactions)
{
_currentTransaction?.Finish(SpanStatus.Ok);
_currentTransaction = null;
}
}
private void OnApplicationOnRequestedThemeChanged(object? sender, AppThemeChangedEventArgs e) =>
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.RequestedThemeChanged), SystemType, RenderingCategory, data => data.Add(nameof(e.RequestedTheme), e.RequestedTheme.ToString()));

Expand All @@ -340,8 +405,15 @@
private void OnWindowOnDeactivated(object? sender, EventArgs _) =>
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Window.Deactivated), SystemType, LifecycleCategory);

private void OnWindowOnStopped(object? sender, EventArgs _) =>
private void OnWindowOnStopped(object? sender, EventArgs _)
{
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Window.Stopped), SystemType, LifecycleCategory);
if (_options.EnableNavigationTransactions)
{
_currentTransaction?.Finish(SpanStatus.Ok);
_currentTransaction = null;
}
}

private void OnWindowOnResumed(object? sender, EventArgs _) =>
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Window.Resumed), SystemType, LifecycleCategory);
Expand Down Expand Up @@ -419,22 +491,37 @@

// Shell Events

private void OnShellOnNavigating(object? sender, ShellNavigatingEventArgs e) =>
private void OnShellOnNavigating(object? sender, ShellNavigatingEventArgs e)
{
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Shell.Navigating), NavigationType, NavigationCategory, data =>
{
data.Add("from", e.Current?.Location.ToString() ?? "");
data.Add("to", e.Target?.Location.ToString() ?? "");
data.Add(nameof(e.Source), e.Source.ToString());
});

private void OnShellOnNavigated(object? sender, ShellNavigatedEventArgs e) =>
if (_options.EnableNavigationTransactions)
{
StartNavigationTransaction(e.Target?.Location.ToString() ?? "Unknown");
}
}

private void OnShellOnNavigated(object? sender, ShellNavigatedEventArgs e)
{
_hub.AddBreadcrumbForEvent(_options, sender, nameof(Shell.Navigated), NavigationType, NavigationCategory, data =>
{
data.Add("from", e.Previous?.Location.ToString() ?? "");
data.Add("to", e.Current?.Location.ToString() ?? "");
data.Add(nameof(e.Source), e.Source.ToString());
});

// Update the transaction name to the final resolved route now that navigation is confirmed
if (_options.EnableNavigationTransactions && _currentTransaction != null)
{
_currentTransaction.Name = e.Current?.Location.ToString() ?? _currentTransaction.Name;
}
}

// Page Events

private void OnPageOnAppearing(object? sender, EventArgs _) =>
Expand Down
17 changes: 17 additions & 0 deletions src/Sentry.Maui/SentryMauiOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,23 @@ public SentryMauiOptions()
/// </remarks>
public bool AttachScreenshot { get; set; }

/// <summary>
/// Automatically starts a Sentry transaction when the user navigates to a new page and sets it on the scope,
/// allowing child spans (e.g. HTTP requests, database calls) to be attached during page load.
/// The transaction finishes automatically after <see cref="NavigationTransactionIdleTimeout"/> if not
/// finished explicitly first (e.g. by a subsequent navigation).
/// Requires <see cref="SentryOptions.TracesSampleRate"/> or <see cref="SentryOptions.TracesSampler"/> to
/// be configured.
/// The default is <c>true</c>.
/// </summary>
public bool EnableNavigationTransactions { get; set; } = true;

/// <summary>
/// Controls how long an automatic navigation transaction waits before finishing itself when not explicitly
/// finished. Defaults to 3 seconds.
/// </summary>
public TimeSpan NavigationTransactionIdleTimeout { get; set; } = TimeSpan.FromSeconds(3);

private Func<SentryEvent, SentryHint, bool>? _beforeCapture;
/// <summary>
/// Action performed before attaching a screenshot
Expand Down
8 changes: 7 additions & 1 deletion src/Sentry/Extensibility/DisabledHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Sentry.Extensibility;
/// <summary>
/// Disabled Hub.
/// </summary>
public class DisabledHub : IHub, IDisposable
public class DisabledHub : IHub, IHubInternal, IDisposable
{
/// <summary>
/// The singleton instance.
Expand Down Expand Up @@ -81,6 +81,12 @@ public void UnsetTag(string key)
public ITransactionTracer StartTransaction(ITransactionContext context,
IReadOnlyDictionary<string, object?> customSamplingContext) => NoOpTransaction.Instance;

/// <summary>
/// Returns a dummy transaction.
/// </summary>
public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout)
=> NoOpTransaction.Instance;

/// <summary>
/// No-Op.
/// </summary>
Expand Down
10 changes: 9 additions & 1 deletion src/Sentry/Extensibility/HubAdapter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Sentry.Infrastructure;
using Sentry.Internal;
using Sentry.Protocol.Envelopes;

namespace Sentry.Extensibility;
Expand All @@ -12,7 +13,7 @@ namespace Sentry.Extensibility;
/// </remarks>
/// <inheritdoc cref="IHub" />
[DebuggerStepThrough]
public sealed class HubAdapter : IHub
public sealed class HubAdapter : IHub, IHubInternal
{
/// <summary>
/// The single instance which forwards all calls to <see cref="SentrySdk"/>
Expand Down Expand Up @@ -121,6 +122,13 @@ internal ITransactionTracer StartTransaction(
DynamicSamplingContext? dynamicSamplingContext)
=> SentrySdk.StartTransaction(context, customSamplingContext, dynamicSamplingContext);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
ITransactionTracer IHubInternal.StartTransaction(ITransactionContext context, TimeSpan? idleTimeout)
=> SentrySdk.StartTransaction(context, idleTimeout);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions src/Sentry/ITransactionTracer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,9 @@ public interface ITransactionTracer : ITransactionData, ISpan
/// Gets the last active (not finished) span in this transaction.
/// </summary>
public ISpan? GetLastActiveSpan();

/// <summary>
/// Resets the idle timeout for auto-finishing transactions. No-op for transactions without an idle timeout.
/// </summary>
public void ResetIdleTimeout();
}
17 changes: 17 additions & 0 deletions src/Sentry/Infrastructure/ITimer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
namespace Sentry.Infrastructure;

/// <summary>
/// Abstraction over a one-shot timer, to allow deterministic testing.
/// </summary>
internal interface ISentryTimer : IDisposable
{
/// <summary>
/// Starts (or restarts) the timer to fire after <paramref name="timeout"/>.
/// </summary>
void Start(TimeSpan timeout);

/// <summary>
/// Cancels any pending fire. Has no effect if the timer is already cancelled.
/// </summary>
void Cancel();
}
22 changes: 22 additions & 0 deletions src/Sentry/Infrastructure/SystemTimer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
namespace Sentry.Infrastructure;

/// <summary>
/// Production <see cref="ISentryTimer"/> backed by <see cref="System.Threading.Timer"/>.
/// </summary>
internal sealed class SystemTimer : ISentryTimer
{
private readonly Timer _timer;

public SystemTimer(Action callback)
{
_timer = new Timer(_ => callback(), null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
}

public void Start(TimeSpan timeout) =>
_timer.Change(timeout, Timeout.InfiniteTimeSpan);

public void Cancel() =>
_timer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);

public void Dispose() => _timer.Dispose();
}
10 changes: 7 additions & 3 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Sentry.Internal;

internal class Hub : IHub, IDisposable
internal class Hub : IHub, IHubInternal, IDisposable
{
private readonly Lock _sessionPauseLock = new();

Expand Down Expand Up @@ -173,10 +173,14 @@ public ITransactionTracer StartTransaction(
IReadOnlyDictionary<string, object?> customSamplingContext)
=> StartTransaction(context, customSamplingContext, null);

public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout)
=> StartTransaction(context, new Dictionary<string, object?>(), null, idleTimeout);

internal ITransactionTracer StartTransaction(
ITransactionContext context,
IReadOnlyDictionary<string, object?> customSamplingContext,
DynamicSamplingContext? dynamicSamplingContext)
DynamicSamplingContext? dynamicSamplingContext,
TimeSpan? idleTimeout = null)
{
// If the hub is disabled, we will always sample out. In other words, starting a transaction
// after disposing the hub will result in that transaction not being sent to Sentry.
Expand Down Expand Up @@ -255,7 +259,7 @@ internal ITransactionTracer StartTransaction(
return unsampledTransaction;
}

var transaction = new TransactionTracer(this, context)
var transaction = new TransactionTracer(this, context, idleTimeout)
{
SampleRate = sampleRate,
SampleRand = sampleRand,
Expand Down
15 changes: 15 additions & 0 deletions src/Sentry/Internal/IHubInternal.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
namespace Sentry.Internal;

/// <summary>
/// Internal hub interface exposing additional overloads not part of the public <see cref="IHub"/> contract.
/// Implemented by <see cref="Hub"/>, <see cref="Extensibility.DisabledHub"/>, and
/// <see cref="Extensibility.HubAdapter"/>.
/// </summary>
internal interface IHubInternal : IHub
{
/// <summary>
/// Starts a transaction that will automatically finish after <paramref name="idleTimeout"/> if not
/// finished explicitly first.
/// </summary>
public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout);
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Technically it'd be a breaking change if we added this method to the public IHub interface. Potentially in v7 we could consider making this public and getting rid of the IHubInternal interface.

}
2 changes: 2 additions & 0 deletions src/Sentry/Internal/NoOpTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,7 @@ public IReadOnlyList<string> Fingerprint

public ISpan? GetLastActiveSpan() => default;

public void ResetIdleTimeout() { }

public void AddBreadcrumb(Breadcrumb breadcrumb) { }
}
10 changes: 10 additions & 0 deletions src/Sentry/SentrySdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,16 @@ internal static ITransactionTracer StartTransaction(
DynamicSamplingContext? dynamicSamplingContext)
=> CurrentHub.StartTransaction(context, customSamplingContext, dynamicSamplingContext);

/// <summary>
/// Starts a transaction that will automatically finish after <paramref name="idleTimeout"/> if not
/// finished explicitly first.
/// </summary>
[DebuggerStepThrough]
internal static ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout)
=> CurrentHub is IHubInternal internalHub
? internalHub.StartTransaction(context, idleTimeout)
: CurrentHub.StartTransaction(context);

/// <summary>
/// Starts a transaction.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions src/Sentry/SpanTracer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@
{
Status ??= SpanStatus.Ok;
EndTimestamp ??= _stopwatch.CurrentDateTimeOffset;
Transaction?.ChildSpanFinished();

Check warning on line 159 in src/Sentry/SpanTracer.cs

View check run for this annotation

@sentry/warden / warden: find-bugs

[GPX-WHB] _manualTransactionOnScope flag is never cleared, permanently disabling auto navigation transactions (additional location)

The comment on lines 368-369 states '_manualTransactionOnScope' should be 'cleared on the next navigation so we re-evaluate', but the flag is never actually reset to false after being set to true. Once a user-created transaction is detected on the scope, the SDK will permanently stop creating automatic navigation transactions for the lifetime of the MauiEventsBinder instance, even after the user's transaction finishes.
}

/// <inheritdoc />
Expand Down
Loading
Loading