Skip to content

[Crashtracking] Improve handling of unhandled exceptions on Windows#6435

Merged
kevingosse merged 3 commits into
masterfrom
kevin/windows_crashes
Dec 17, 2024
Merged

[Crashtracking] Improve handling of unhandled exceptions on Windows#6435
kevingosse merged 3 commits into
masterfrom
kevin/windows_crashes

Conversation

@kevingosse

@kevingosse kevingosse commented Dec 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary of changes

In crashtracking on windows, go the extra mile to dispose the ClrMD DataTarget when an unhandled exception occurs.

Reason for change

A lot of the logic is implemented on the native side (Datadog.Profiler.Native and libdatadog). Unfortunately, native exceptions such as access violation can't be caught on the managed side. It means that if the native side crashes, we don't have a chance to clean the ClrMD context, which in turn means that the threads of the crashing process will remain suspended, preventing it from exiting. To prevent that, we bypass .NET exception management and use SetUnhandledExceptionFilter to catch all exceptions.

Implementation details

Use SetUnhandledExceptionFilter to get a chance to run code when an exception is unhandled. After disposing the ClrMD DataTarget, we call the old unhandled exception filter if any.

Test coverage

Added a test to confirm the process doesn't remain frozen when dd-dotnet crashes.

@andrewlock

andrewlock commented Dec 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6435) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (68ms)  : 64, 73
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (6435) - mean (979ms)  : 953, 1005
     .   : milestone, 979,
    master - mean (974ms)  : 948, 999
     .   : milestone, 974,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6435) - mean (108ms)  : 105, 110
     .   : milestone, 108,
    master - mean (106ms)  : 104, 108
     .   : milestone, 106,

    section CallTarget+Inlining+NGEN
    This PR (6435) - mean (674ms)  : 657, 692
     .   : milestone, 674,
    master - mean (674ms)  : 660, 689
     .   : milestone, 674,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6435) - mean (91ms)  : 89, 93
     .   : milestone, 91,
    master - mean (90ms)  : 88, 92
     .   : milestone, 90,

    section CallTarget+Inlining+NGEN
    This PR (6435) - mean (634ms)  : 620, 649
     .   : milestone, 634,
    master - mean (629ms)  : 614, 644
     .   : milestone, 629,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6435) - mean (195ms)  : 190, 200
     .   : milestone, 195,
    master - mean (194ms)  : 190, 198
     .   : milestone, 194,

    section CallTarget+Inlining+NGEN
    This PR (6435) - mean (1,105ms)  : 1076, 1134
     .   : milestone, 1105,
    master - mean (1,098ms)  : 1074, 1123
     .   : milestone, 1098,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6435) - mean (278ms)  : 274, 283
     .   : milestone, 278,
    master - mean (277ms)  : 273, 281
     .   : milestone, 277,

    section CallTarget+Inlining+NGEN
    This PR (6435) - mean (870ms)  : 841, 899
     .   : milestone, 870,
    master - mean (869ms)  : 838, 901
     .   : milestone, 869,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6435) - mean (267ms)  : 263, 271
     .   : milestone, 267,
    master - mean (266ms)  : 262, 271
     .   : milestone, 266,

    section CallTarget+Inlining+NGEN
    This PR (6435) - mean (852ms)  : 816, 887
     .   : milestone, 852,
    master - mean (850ms)  : 811, 890
     .   : milestone, 850,
Loading

@datadog-ddstaging

datadog-ddstaging Bot commented Dec 13, 2024 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: kevin/windows_crashes
Commit report: 7574b66
Test service: dd-trace-dotnet

✅ 0 Failed, 460183 Passed, 2846 Skipped, 19h 51m 55.84s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

  • FlakyRetries - Datadog.Trace.ClrProfiler.IntegrationTests.CI.MsTestV2RetriesTests - Last Failure

    Expand for error 
     The sample did not exit in 600000ms. Memory dump taken: True. Killing process.

@kevingosse
kevingosse marked this pull request as ready for review December 13, 2024 14:30
@kevingosse
kevingosse requested a review from a team as a code owner December 13, 2024 14:30
@andrewlock

andrewlock commented Dec 13, 2024

Copy link
Copy Markdown
Member

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6435) (11.120M)   : 0, 11119889
    master (11.195M)   : 0, 11194535
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6435) (7.244M)   : 0, 7243673
    master (7.216M)   : 0, 7216361
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.611M)   : 0, 7610504

    section Manual
    master (11.163M)   : 0, 11162548

    section Manual + Automatic
    This PR (6435) (6.676M)   : 0, 6675757
    master (6.753M)   : 0, 6752572

    section DD_TRACE_ENABLED=0
    master (10.216M)   : 0, 10215642
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6435) (9.611M)   : 0, 9610543
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6435) (6.333M)   : 0, 6332628

    section Manual + Automatic
    This PR (6435) (5.883M)   : 0, 5882570
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6435) (9.660M)   : 0, 9660075
    master (9.988M)   : 0, 9987589
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6435) (6.183M)   : 0, 6182868
    master (6.338M)   : 0, 6338249
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (7.137M)   : 0, 7136842

    section Manual
    master (9.965M)   : 0, 9965094

    section Manual + Automatic
    This PR (6435) (5.603M)   : 0, 5602818
    master (5.791M)   : 0, 5790893

    section DD_TRACE_ENABLED=0
    master (9.274M)   : 0, 9274318
Loading

@andrewlock

andrewlock commented Dec 13, 2024

Copy link
Copy Markdown
Member

Benchmarks Report for tracer 🐌

Benchmarks for #6435 compared to master:

  • 3 benchmarks are faster, with geometric mean 1.132
  • 1 benchmarks are slower, with geometric mean 1.116
  • All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 8μs 44.8ns 297ns 0.0159 0.00794 0 5.61 KB
master StartStopWithChild netcoreapp3.1 9.82μs 47.8ns 203ns 0.0242 0.00969 0 5.8 KB
master StartStopWithChild net472 16.2μs 49.4ns 191ns 1.04 0.3 0.0974 6.21 KB
#6435 StartStopWithChild net6.0 8.09μs 44.9ns 270ns 0.0128 0.00427 0 5.6 KB
#6435 StartStopWithChild netcoreapp3.1 10.1μs 50.1ns 296ns 0.0263 0.0105 0 5.8 KB
#6435 StartStopWithChild net472 16.3μs 47.7ns 178ns 1.04 0.308 0.0998 6.22 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 480μs 735ns 2.85μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 647μs 309ns 1.2μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 853μs 419ns 1.62μs 0.425 0 0 3.3 KB
#6435 WriteAndFlushEnrichedTraces net6.0 503μs 245ns 885ns 0 0 0 2.7 KB
#6435 WriteAndFlushEnrichedTraces netcoreapp3.1 651μs 317ns 1.23μs 0 0 0 2.7 KB
#6435 WriteAndFlushEnrichedTraces net472 851μs 469ns 1.82μs 0.422 0 0 3.3 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 150μs 900ns 8.91μs 0.136 0 0 14.47 KB
master SendRequest netcoreapp3.1 169μs 1.02μs 9.81μs 0.181 0 0 17.27 KB
master SendRequest net472 0.00166ns 0.000575ns 0.00215ns 0 0 0 0 b
#6435 SendRequest net6.0 153μs 896ns 8.55μs 0.147 0 0 14.47 KB
#6435 SendRequest netcoreapp3.1 165μs 944ns 8.76μs 0.152 0 0 17.27 KB
#6435 SendRequest net472 0.00195ns 0.000743ns 0.00288ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 570μs 3.09μs 18μs 0.579 0 0 41.64 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 677μs 3.69μs 21.2μs 0.347 0 0 41.78 KB
master WriteAndFlushEnrichedTraces net472 833μs 3.19μs 11.9μs 8.33 2.5 0.417 53.26 KB
#6435 WriteAndFlushEnrichedTraces net6.0 575μs 3.1μs 16.1μs 0.553 0 0 41.57 KB
#6435 WriteAndFlushEnrichedTraces netcoreapp3.1 668μs 3.87μs 33.9μs 0.334 0 0 41.72 KB
#6435 WriteAndFlushEnrichedTraces net472 845μs 2.74μs 10.6μs 8.87 2.53 0.422 53.29 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.3μs 0.717ns 2.78ns 0.0142 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.76μs 1.82ns 7.04ns 0.0139 0 0 1.02 KB
master ExecuteNonQuery net472 2.01μs 1.64ns 6.34ns 0.156 0.00101 0 987 B
#6435 ExecuteNonQuery net6.0 1.34μs 1.94ns 7.51ns 0.0147 0 0 1.02 KB
#6435 ExecuteNonQuery netcoreapp3.1 1.75μs 1.8ns 6.97ns 0.0132 0 0 1.02 KB
#6435 ExecuteNonQuery net472 2.11μs 1.25ns 4.66ns 0.157 0.00105 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6435

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch‑net6.0 1.137 1,269.04 1,115.98
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync‑netcoreapp3.1 1.116 1,741.81 1,560.39

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.27μs 0.541ns 2.1ns 0.0133 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.59μs 0.682ns 2.64ns 0.0128 0 0 976 B
master CallElasticsearch net472 2.59μs 1.04ns 3.88ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.39μs 0.653ns 2.53ns 0.0132 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.74μs 0.608ns 2.27ns 0.0139 0 0 1.02 KB
master CallElasticsearchAsync net472 2.58μs 0.841ns 3.15ns 0.167 0 0 1.05 KB
#6435 CallElasticsearch net6.0 1.12μs 0.568ns 2.12ns 0.014 0 0 976 B
#6435 CallElasticsearch netcoreapp3.1 1.59μs 4.56ns 17.6ns 0.0135 0 0 976 B
#6435 CallElasticsearch net472 2.5μs 1.21ns 4.35ns 0.157 0 0 995 B
#6435 CallElasticsearchAsync net6.0 1.26μs 0.606ns 2.27ns 0.0133 0 0 952 B
#6435 CallElasticsearchAsync netcoreapp3.1 1.56μs 0.787ns 2.94ns 0.0135 0 0 1.02 KB
#6435 CallElasticsearchAsync net472 2.54μs 1.06ns 3.98ns 0.166 0 0 1.05 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.31μs 3.72ns 14.4ns 0.0129 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.72μs 0.578ns 2.16ns 0.0129 0 0 952 B
master ExecuteAsync net472 1.79μs 0.544ns 1.96ns 0.145 0 0 915 B
#6435 ExecuteAsync net6.0 1.18μs 0.766ns 2.87ns 0.0136 0 0 952 B
#6435 ExecuteAsync netcoreapp3.1 1.59μs 0.684ns 2.65ns 0.0127 0 0 952 B
#6435 ExecuteAsync net472 1.83μs 0.46ns 1.78ns 0.145 0 0 915 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.37μs 1.03ns 3.99ns 0.0326 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.27μs 2.36ns 9.13ns 0.0396 0 0 2.85 KB
master SendAsync net472 7.34μs 1.57ns 5.87ns 0.495 0 0 3.12 KB
#6435 SendAsync net6.0 4.27μs 1.49ns 5.75ns 0.0321 0 0 2.31 KB
#6435 SendAsync netcoreapp3.1 5.23μs 2.22ns 8.01ns 0.0393 0 0 2.85 KB
#6435 SendAsync net472 7.41μs 2.05ns 7.94ns 0.496 0 0 3.12 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 1.48μs 2.98ns 11.6ns 0.0234 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.2μs 1.92ns 7.43ns 0.022 0 0 1.64 KB
master EnrichedLog net472 2.71μs 1.8ns 6.48ns 0.249 0 0 1.57 KB
#6435 EnrichedLog net6.0 1.46μs 0.6ns 2.32ns 0.0228 0 0 1.64 KB
#6435 EnrichedLog netcoreapp3.1 2.12μs 4.2ns 16.3ns 0.022 0 0 1.64 KB
#6435 EnrichedLog net472 2.76μs 1.07ns 4.14ns 0.249 0 0 1.57 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 119μs 269ns 1.04μs 0.0594 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 124μs 258ns 1μs 0 0 0 4.28 KB
master EnrichedLog net472 151μs 250ns 970ns 0.669 0.223 0 4.46 KB
#6435 EnrichedLog net6.0 119μs 152ns 570ns 0 0 0 4.28 KB
#6435 EnrichedLog netcoreapp3.1 124μs 165ns 638ns 0 0 0 4.28 KB
#6435 EnrichedLog net472 153μs 97.7ns 379ns 0.685 0.228 0 4.46 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 3.13μs 1.35ns 5.22ns 0.0312 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.18μs 2.66ns 10.3ns 0.0291 0 0 2.2 KB
master EnrichedLog net472 4.9μs 0.728ns 2.72ns 0.319 0 0 2.02 KB
#6435 EnrichedLog net6.0 2.95μs 1.78ns 6.89ns 0.0313 0 0 2.2 KB
#6435 EnrichedLog netcoreapp3.1 4.18μs 2.09ns 8.09ns 0.0291 0 0 2.2 KB
#6435 EnrichedLog net472 4.99μs 1.9ns 7.36ns 0.32 0 0 2.02 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.32μs 1.55ns 5.98ns 0.0158 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.83μs 1.73ns 6.7ns 0.0153 0 0 1.14 KB
master SendReceive net472 2.11μs 1.3ns 5.03ns 0.184 0 0 1.16 KB
#6435 SendReceive net6.0 1.37μs 0.705ns 2.73ns 0.0158 0 0 1.14 KB
#6435 SendReceive netcoreapp3.1 1.81μs 0.791ns 3.06ns 0.0153 0 0 1.14 KB
#6435 SendReceive net472 2.14μs 1.01ns 3.9ns 0.183 0 0 1.16 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.74μs 2.57ns 9.95ns 0.022 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.81μs 1.28ns 4.78ns 0.021 0 0 1.65 KB
master EnrichedLog net472 4.44μs 0.633ns 2.19ns 0.324 0 0 2.04 KB
#6435 EnrichedLog net6.0 2.7μs 0.706ns 2.74ns 0.0217 0 0 1.6 KB
#6435 EnrichedLog netcoreapp3.1 3.98μs 2.12ns 8.22ns 0.0216 0 0 1.65 KB
#6435 EnrichedLog net472 4.3μs 3.14ns 11.7ns 0.324 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6435

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 1.116 398.31 444.35

Faster 🎉 in #6435

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0 1.144 559.56 489.05

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 398ns 0.261ns 0.977ns 0.00814 0 0 576 B
master StartFinishSpan netcoreapp3.1 589ns 0.702ns 2.72ns 0.00767 0 0 576 B
master StartFinishSpan net472 649ns 0.436ns 1.69ns 0.0918 0 0 578 B
master StartFinishScope net6.0 560ns 0.241ns 0.901ns 0.00982 0 0 696 B
master StartFinishScope netcoreapp3.1 723ns 0.791ns 3.06ns 0.0094 0 0 696 B
master StartFinishScope net472 875ns 0.528ns 2.04ns 0.104 0 0 658 B
#6435 StartFinishSpan net6.0 444ns 0.4ns 1.55ns 0.00809 0 0 576 B
#6435 StartFinishSpan netcoreapp3.1 636ns 0.469ns 1.82ns 0.00796 0 0 576 B
#6435 StartFinishSpan net472 650ns 0.27ns 1.05ns 0.0918 0 0 578 B
#6435 StartFinishScope net6.0 489ns 0.518ns 2.01ns 0.0098 0 0 696 B
#6435 StartFinishScope netcoreapp3.1 761ns 0.699ns 2.71ns 0.00909 0 0 696 B
#6435 StartFinishScope net472 829ns 1.04ns 4.04ns 0.105 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 609ns 0.524ns 2.03ns 0.00977 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 927ns 0.491ns 1.9ns 0.00934 0 0 696 B
master RunOnMethodBegin net472 1.12μs 1.19ns 4.47ns 0.104 0 0 658 B
#6435 RunOnMethodBegin net6.0 607ns 0.377ns 1.41ns 0.00973 0 0 696 B
#6435 RunOnMethodBegin netcoreapp3.1 914ns 0.599ns 2.32ns 0.00919 0 0 696 B
#6435 RunOnMethodBegin net472 1.18μs 0.602ns 2.25ns 0.104 0 0 658 B

andrewlock
andrewlock approved these changes Dec 17, 2024
View reviewed changes
Comment thread tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs
return;
}

if (Environment.GetEnvironmentVariable("DD_INTERNAL_CRASHTRACKING_CRASH") == "1")

@andrewlock andrewlock Dec 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wow, you added a real variable that causes crashes into the production code. Bold move.

@kevingosse kevingosse Dec 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

:thuglife:

Comment thread tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs
// To prevent that, we bypass .NET exception management and use SetUnhandledExceptionFilter to catch all exceptions.

// We're running managed code in a completely unsupported state, so keep the amount of work to an absolute minimum.
_dataTarget?.Dispose();

@andrewlock andrewlock Dec 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any way this could have already been disposed? And what happens if so?

@kevingosse kevingosse Dec 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's fine, there's a boolean check: https://github.com/microsoft/clrmd/blob/main/src/Microsoft.Diagnostics.Runtime/DataTarget.cs#L88-L103

Comment thread tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs
enableProfiler: true,
[LdPreloadConfig, CrashReportConfig(reportFile), ("DD_INTERNAL_CRASHTRACKING_CRASH", "1")]);

var completion = await Task.WhenAny(helper.Task, Task.Delay(TimeSpan.FromMinutes(1)));

@andrewlock andrewlock Dec 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One minute seems like a long time, but, CI... we use 10 minutes elsewhere. Just sayin...

Comment thread tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs
Comment on lines +377 to +378
SkipOn.Platform(SkipOn.PlatformValue.MacOs);
SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86);

@andrewlock andrewlock Dec 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this works on Linux then too? Even though kernel32.dll?

@kevingosse kevingosse Dec 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On Linux we should have the same requirements (= the crashing process shouldn't remain frozen if dd-dotnet crashes). It just happens that we don't need SetUnhandledExceptionFilter for that, Linux automatically resumes the suspended threads when dd-dotnet crashes.

You may then think "but wait, why are the installer tests failing then?". Yup, I know right?

@kevingosse
kevingosse merged commit f5dcda4 into master Dec 17, 2024
@kevingosse
kevingosse deleted the kevin/windows_crashes branch December 17, 2024 11:46
@github-actions github-actions Bot added this to the vNext-v3 milestone Dec 17, 2024
chojomok pushed a commit that referenced this pull request Jul 15, 2025
@chojomok
[Crashtracking] Improve handling of unhandled exceptions on Windows (#…
cf9853a 
…6435)

## Summary of changes

In crashtracking on windows, go the extra mile to dispose the ClrMD
`DataTarget` when an unhandled exception occurs.

## Reason for change

A lot of the logic is implemented on the native side
(Datadog.Profiler.Native and libdatadog). Unfortunately, native
exceptions such as access violation can't be caught on the managed side.
It means that if the native side crashes, we don't have a chance to
clean the ClrMD context, which in turn means that the threads of the
crashing process will remain suspended, preventing it from exiting. To
prevent that, we bypass .NET exception management and use
SetUnhandledExceptionFilter to catch all exceptions.

## Implementation details

Use `SetUnhandledExceptionFilter` to get a chance to run code when an
exception is unhandled. After disposing the ClrMD `DataTarget`, we call
the old unhandled exception filter if any.

## Test coverage

Added a test to confirm the process doesn't remain frozen when dd-dotnet
crashes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrewlock andrewlock andrewlock approved these changes

Assignees

No one assigned

Labels

area:crashtracking type:reliability

Projects

None yet

Milestone

3.7.0

Development

Successfully merging this pull request may close these issues.

2 participants

@kevingosse @andrewlock