Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ASM] Fix our legacy encoder benchmarks memory leak #5308

Merged
merged 13 commits into from
Mar 28, 2024
2 changes: 0 additions & 2 deletions tracer/missing-nullability-files.csv
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,6 @@ src/Datadog.Trace/AppSec/Concurrency/ReaderWriterLock.Framework.cs
src/Datadog.Trace/AppSec/Waf/IWaf.cs
src/Datadog.Trace/AppSec/Waf/WafConstants.cs
src/Datadog.Trace/AppSec/Waf/WafReturnCode.cs
src/Datadog.Trace/AppSec/WafEncoding/Obj.cs
src/Datadog.Trace/AppSec/WafEncoding/ObjType.cs
src/Datadog.Trace/Ci/Agent/ApmAgentWriter.cs
src/Datadog.Trace/Ci/Agent/CIVisibilityProtocolWriter.cs
src/Datadog.Trace/Ci/Agent/CIWriterFileSender.cs
Expand Down
38 changes: 22 additions & 16 deletions tracer/src/Datadog.Trace/AppSec/Waf/Context.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal class Context : IContext

private readonly Waf _waf;

private readonly List<IEncodeResult> _argCache;
private readonly List<IEncodeResult> _encodeResults;
private readonly Stopwatch _stopwatch;
private readonly WafLibraryInvoker _wafLibraryInvoker;
private readonly IEncoder _encoder;
Expand All @@ -41,7 +41,7 @@ private Context(IntPtr contextHandle, Waf waf, WafLibraryInvoker wafLibraryInvok
_wafLibraryInvoker = wafLibraryInvoker;
_encoder = encoder;
_stopwatch = new Stopwatch();
_argCache = new(64);
_encodeResults = new(64);
}

~Context() => Dispose(false);
Expand All @@ -64,7 +64,7 @@ private Context(IntPtr contextHandle, Waf waf, WafLibraryInvoker wafLibraryInvok
public IResult? RunWithEphemeral(IDictionary<string, object> ephemeralAddressData, ulong timeoutMicroSeconds)
=> RunInternal(null, ephemeralAddressData, timeoutMicroSeconds);

private IResult? RunInternal(IDictionary<string, object>? persistentAddressData, IDictionary<string, object>? ephemeralAddressData, ulong timeoutMicroSeconds)
private unsafe IResult? RunInternal(IDictionary<string, object>? persistentAddressData, IDictionary<string, object>? ephemeralAddressData, ulong timeoutMicroSeconds)
{
if (_disposed)
{
Expand Down Expand Up @@ -100,29 +100,35 @@ private Context(IntPtr contextHandle, Waf waf, WafLibraryInvoker wafLibraryInvok
// Calling _encoder.Encode(null) results in a null object that will cause the WAF to error
// The WAF can be called with an empty dictionary (though we should avoid doing this).

var pwPersistentArgs = IntPtr.Zero;
if (persistentAddressData != null)
DdwafObjectStruct pwPersistentArgs = default;
DdwafObjectStruct pwEphemeralArgsValue = default;

if (persistentAddressData is not null)
{
var persistentArgs = _encoder.Encode(persistentAddressData, applySafetyLimits: true);
pwPersistentArgs = persistentArgs.Result;
_argCache.Add(persistentArgs);
pwPersistentArgs = persistentArgs.ResultDdwafObject;
_encodeResults.Add(persistentArgs);
}

// pwEphemeralArgs follow a different lifecycle and should be disposed immediately
using var ephemeralArgs =
ephemeralAddressData is { Count: > 0 }
? _encoder.Encode(ephemeralAddressData, applySafetyLimits: true)
: null;
var pwEphemeralArgs = ephemeralArgs?.Result ?? IntPtr.Zero;
using var ephemeralArgs = ephemeralAddressData is { Count: > 0 }
? _encoder.Encode(ephemeralAddressData, applySafetyLimits: true)
: null;

if (pwPersistentArgs == IntPtr.Zero && pwEphemeralArgs == IntPtr.Zero)
if (persistentAddressData is null && ephemeralArgs is null)
{
Log.Error("Both pwPersistentArgs and pwEphemeralArgs are null");
return null;
}

if (ephemeralArgs is not null)
{
// WARNING: Don't use ref here, we need to make a copy because ephemeralArgs is on the heap
pwEphemeralArgsValue = ephemeralArgs.ResultDdwafObject;
}

// WARNING: DO NOT DISPOSE pwPersistentArgs until the end of this class's lifecycle, i.e in the dispose. Otherwise waf might crash with fatal exception.
code = _waf.Run(_contextHandle, pwPersistentArgs, pwEphemeralArgs, ref retNative, timeoutMicroSeconds);
code = _waf.Run(_contextHandle, persistentAddressData != null ? &pwPersistentArgs : null, ephemeralArgs != null ? &pwEphemeralArgsValue : null, ref retNative, timeoutMicroSeconds);
}

_stopwatch.Stop();
Expand Down Expand Up @@ -151,9 +157,9 @@ public void Dispose(bool disposing)
_disposed = true;

// WARNING do not move this above, this should only be disposed in the end of the context's life
foreach (var arg in _argCache)
foreach (var encodeResult in _encodeResults)
{
arg.Dispose();
encodeResult.Dispose();
}

lock (_stopwatch)
Expand Down
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace/AppSec/Waf/IWaf.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal interface IWaf : IDisposable

public IContext CreateContext();

internal WafReturnCode Run(IntPtr contextHandle, IntPtr rawPersistentData, IntPtr rawEphemeralData, ref DdwafResultStruct retNative, ulong timeoutMicroSeconds);
internal unsafe WafReturnCode Run(IntPtr contextHandle, DdwafObjectStruct* rawPersistentData, DdwafObjectStruct* rawEphemeralData, ref DdwafResultStruct retNative, ulong timeoutMicroSeconds);

UpdateResult UpdateWafFromConfigurationStatus(ConfigurationStatus configurationStatus);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ private static void LogRuleDetailsIfDebugEnabled(JToken root)
return root;
}

internal InitResult Configure(IntPtr rulesObj, IEncoder encoder, DdwafConfigStruct configStruct, ref DdwafObjectStruct diagnostics, string? rulesFile)
internal InitResult Configure(ref DdwafObjectStruct rulesObj, IEncoder encoder, DdwafConfigStruct configStruct, ref DdwafObjectStruct diagnostics, string? rulesFile)
{
var wafHandle = _wafLibraryInvoker.Init(rulesObj, ref configStruct, ref diagnostics);
var wafHandle = _wafLibraryInvoker.Init(ref rulesObj, ref configStruct, ref diagnostics);
if (wafHandle == IntPtr.Zero)
{
Log.Warning("DDAS-0005-00: WAF initialization failed.");
Expand Down
Loading
Loading