Skip to content

Waf memory leak fix#3237

Merged
dromanol merged 1 commit into
masterfrom
dani/asm/waf_leak_fix
Sep 21, 2022
Merged

Waf memory leak fix#3237
dromanol merged 1 commit into
masterfrom
dani/asm/waf_leak_fix

Conversation

@dromanol

@dromanol dromanol commented Sep 21, 2022

Copy link
Copy Markdown
Contributor

Summary of changes

Added missing FreeObjet function to native waf init call

Reason for change

Detected memory leak in appsec enabled applications

Implementation details

Waf v1.5.0 changed the init API, moving the free function argument from CreateContext to Init function. The line passing the Free function in the call to Init was "missing"

Test coverage

Other details

@andrewlock

andrewlock commented Sep 21, 2022

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #3237 compared to master:

  • All benchmarks have the same speed
  • 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.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net472 713μs 386ns 1.44μs 0.355 0 0 3.18 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 461μs 156ns 582ns 0 0 0 2.58 KB
#3237 WriteAndFlushEnrichedTraces net472 710μs 499ns 1.93μs 0.359 0 0 3.18 KB
#3237 WriteAndFlushEnrichedTraces netcoreapp3.1 458μs 533ns 2.07μs 0 0 0 2.58 KB
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net472 191ns 0.16ns 0.578ns 0.0675 0 0 425 B
master AllCycleSimpleBody netcoreapp3.1 235ns 0.266ns 0.994ns 0.00587 0 0 424 B
master AllCycleMoreComplexBody net472 180ns 0.159ns 0.614ns 0.0638 0 0 401 B
master AllCycleMoreComplexBody netcoreapp3.1 236ns 0.217ns 0.841ns 0.00545 0 0 400 B
master BodyExtractorSimpleBody net472 259ns 0.321ns 1.2ns 0.0573 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 219ns 0.17ns 0.658ns 0.00374 0 0 272 B
master BodyExtractorMoreComplexBody net472 14.7μs 11.4ns 42.5ns 1.2 0.0219 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 11.9μs 10.8ns 40.4ns 0.0894 0 0 6.75 KB
#3237 AllCycleSimpleBody net472 185ns 0.123ns 0.462ns 0.0675 0 0 425 B
#3237 AllCycleSimpleBody netcoreapp3.1 234ns 0.19ns 0.711ns 0.00585 0 0 424 B
#3237 AllCycleMoreComplexBody net472 180ns 0.192ns 0.745ns 0.0637 0 0 401 B
#3237 AllCycleMoreComplexBody netcoreapp3.1 236ns 0.175ns 0.677ns 0.00553 0 0 400 B
#3237 BodyExtractorSimpleBody net472 278ns 0.234ns 0.905ns 0.0573 0 0 361 B
#3237 BodyExtractorSimpleBody netcoreapp3.1 211ns 0.259ns 0.934ns 0.0037 0 0 272 B
#3237 BodyExtractorMoreComplexBody net472 14.5μs 8.31ns 30ns 1.2 0.0216 0 7.62 KB
#3237 BodyExtractorMoreComplexBody netcoreapp3.1 12.9μs 12.1ns 45.3ns 0.0898 0 0 6.75 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 net472 0ns 0ns 0ns 0 0 0 0 b
master SendRequest netcoreapp3.1 179μs 134ns 517ns 0.268 0 0 20.6 KB
#3237 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3237 SendRequest netcoreapp3.1 182μs 208ns 807ns 0.18 0 0 20.58 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 net472 1.86μs 1.05ns 4.08ns 0.15 0.000939 0 947 B
master ExecuteNonQuery netcoreapp3.1 1.39μs 0.796ns 2.98ns 0.0127 0 0 936 B
#3237 ExecuteNonQuery net472 1.77μs 0.678ns 2.54ns 0.15 0 0 947 B
#3237 ExecuteNonQuery netcoreapp3.1 1.45μs 0.507ns 1.9ns 0.0123 0 0 936 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net472 2.59μs 2.43ns 9.43ns 0.184 0.00129 0 1.16 KB
master CallElasticsearch netcoreapp3.1 1.53μs 0.414ns 1.49ns 0.0145 0 0 1.1 KB
master CallElasticsearchAsync net472 2.66μs 2.09ns 8.11ns 0.205 0 0 1.29 KB
master CallElasticsearchAsync netcoreapp3.1 1.64μs 0.735ns 2.65ns 0.0164 0 0 1.22 KB
#3237 CallElasticsearch net472 2.66μs 2.98ns 11.5ns 0.183 0 0 1.16 KB
#3237 CallElasticsearch netcoreapp3.1 1.53μs 0.732ns 2.83ns 0.0152 0 0 1.1 KB
#3237 CallElasticsearchAsync net472 2.77μs 1.32ns 5.11ns 0.205 0 0 1.29 KB
#3237 CallElasticsearchAsync netcoreapp3.1 1.62μs 0.601ns 2.17ns 0.0163 0 0 1.22 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 net472 2.87μs 4.48ns 17.3ns 0.225 0 0 1.42 KB
master ExecuteAsync netcoreapp3.1 1.8μs 0.769ns 2.98ns 0.018 0 0 1.34 KB
#3237 ExecuteAsync net472 2.86μs 1.54ns 5.75ns 0.225 0 0 1.42 KB
#3237 ExecuteAsync netcoreapp3.1 1.76μs 0.551ns 2.06ns 0.0184 0 0 1.34 KB
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net472 5.9μs 2.99ns 11.2ns 0.44 0 0 2.77 KB
master SendAsync netcoreapp3.1 3.77μs 1.82ns 7.05ns 0.0358 0 0 2.6 KB
#3237 SendAsync net472 5.75μs 22.5ns 87.3ns 0.438 0 0 2.77 KB
#3237 SendAsync netcoreapp3.1 3.65μs 10.1ns 37.9ns 0.0355 0 0 2.6 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 net472 3.25μs 15.5ns 59.8ns 0.287 0 0 1.81 KB
master EnrichedLog netcoreapp3.1 2.48μs 13.5ns 109ns 0.0242 0 0 1.85 KB
#3237 EnrichedLog net472 3.18μs 2.39ns 8.63ns 0.287 0.00157 0 1.81 KB
#3237 EnrichedLog netcoreapp3.1 2.61μs 1.59ns 5.73ns 0.0252 0.00133 0 1.85 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 net472 152μs 199ns 770ns 0.682 0.227 0 4.65 KB
master EnrichedLog netcoreapp3.1 119μs 138ns 535ns 0 0 0 4.49 KB
#3237 EnrichedLog net472 153μs 91.3ns 353ns 0.688 0.229 0 4.66 KB
#3237 EnrichedLog netcoreapp3.1 118μs 252ns 977ns 0.0582 0 0 4.49 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 net472 5.73μs 12.3ns 47.6ns 0.57 0.00292 0 3.59 KB
master EnrichedLog netcoreapp3.1 4.35μs 1.73ns 6.48ns 0.0531 0 0 3.91 KB
#3237 EnrichedLog net472 5.99μs 5.29ns 20.5ns 0.57 0.00298 0 3.59 KB
#3237 EnrichedLog netcoreapp3.1 4.42μs 6.32ns 24.5ns 0.0527 0.00219 0 3.91 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 net472 2.23μs 2.36ns 9.15ns 0.217 0 0 1.37 KB
master SendReceive netcoreapp3.1 1.82μs 0.922ns 3.57ns 0.0174 0 0 1.32 KB
#3237 SendReceive net472 2.27μs 2.03ns 7.59ns 0.218 0 0 1.37 KB
#3237 SendReceive netcoreapp3.1 1.83μs 1.04ns 4.02ns 0.0174 0 0 1.32 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 net472 5.13μs 1.7ns 6.37ns 0.353 0 0 2.23 KB
master EnrichedLog netcoreapp3.1 4.33μs 1.36ns 5.08ns 0.0239 0 0 1.8 KB
#3237 EnrichedLog net472 5.02μs 4.37ns 16.9ns 0.353 0 0 2.23 KB
#3237 EnrichedLog netcoreapp3.1 4.46μs 1.8ns 6.22ns 0.0246 0 0 1.8 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net472 1.24μs 0.732ns 2.74ns 0.129 0 0 810 B
master StartFinishSpan netcoreapp3.1 966ns 1.16ns 4.18ns 0.01 0 0 760 B
master StartFinishScope net472 1.41μs 1.06ns 4.12ns 0.141 0 0 891 B
master StartFinishScope netcoreapp3.1 1.05μs 0.383ns 1.38ns 0.0119 0 0 880 B
#3237 StartFinishSpan net472 1.18μs 0.753ns 2.92ns 0.128 0 0 810 B
#3237 StartFinishSpan netcoreapp3.1 1E+03ns 0.377ns 1.41ns 0.01 0 0 760 B
#3237 StartFinishScope net472 1.47μs 0.837ns 3.13ns 0.141 0 0 891 B
#3237 StartFinishScope netcoreapp3.1 1.09μs 0.454ns 1.76ns 0.0119 0 0 880 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 net472 1.51μs 1.08ns 4.17ns 0.141 0 0 891 B
master RunOnMethodBegin netcoreapp3.1 1.16μs 0.387ns 1.45ns 0.0122 0 0 880 B
#3237 RunOnMethodBegin net472 1.48μs 1.18ns 4.59ns 0.141 0 0 891 B
#3237 RunOnMethodBegin netcoreapp3.1 1.2μs 0.559ns 2.17ns 0.012 0 0 880 B

@andrewlock

andrewlock commented Sep 21, 2022

Copy link
Copy Markdown
Member

Code Coverage Report 📊

⛔ Merging #3237 into master will will decrease line coverage by 5%
⚠️ Merging #3237 into master will will decrease branch coverage by 3%
⛔ Merging #3237 into master will will increase complexity by 14

master #3237 Change
Lines 17963 / 24889 16759 / 24904
Lines % 72% 67% -5% ⛔
Branches 10472 / 15254 9993 / 15270
Branches % 69% 65% -3% ⚠️
Complexity 16477 16491 14 ⛔

View the full report for further details:

Datadog.Trace Breakdown ⛔

master #3237 Change
Lines % 72% 67% -5% ⛔
Branches % 69% 65% -3% ⚠️
Complexity 16477 16491 14 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Debugger.PInvoke.NativeMethodProbeDefinition -100% ⛔ -100% ⛔ 0 ✔️
Datadog.Trace.Debugger.Sink.SnapshotSink -100% ⛔ -100% ⛔ 0 ✔️
Datadog.Trace.Debugger.Snapshots.LazySnapshotSerializerFieldsAndPropsSelector -100% ⛔ -100% ⛔ 0 ✔️
Datadog.Trace.Debugger.Snapshots.DebuggerSnapshotCreator -100% ⛔ -68% ⛔ 1 ⛔
BoundLineProbeLocation -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.CapturedLines -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Instrumentation.DebuggerReturn -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Instrumentation.MethodMetadataInfo -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Models.LineProbeResolveResult -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.PInvoke.NativeLineProbeDefinition -100% ⛔ 0% ✔️ 0 ✔️
...And 48 more

View the full reports for further details:

@dromanol
dromanol marked this pull request as ready for review September 21, 2022 11:11
@dromanol
dromanol requested review from a team as code owners September 21, 2022 11:11
robertpi
robertpi approved these changes Sep 21, 2022
View reviewed changes

@robertpi robertpi left a comment

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.

LGTM

@dromanol dromanol changed the title Added missing free function in waf init Waf memory leak fix Sep 21, 2022
@dromanol
dromanol merged commit 6001db0 into master Sep 21, 2022
@dromanol
dromanol deleted the dani/asm/waf_leak_fix branch September 21, 2022 11:23
@github-actions github-actions Bot added this to the vNext milestone Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robertpi robertpi robertpi approved these changes

Assignees

No one assigned

Labels

area:asm

Projects

None yet

Milestone

2.15.0

Development

Successfully merging this pull request may close these issues.

3 participants

@dromanol @andrewlock @robertpi