Skip to content

Commit fd3c397

Browse files
elinor-funghez2010
authored andcommitted
[cdac] Clear cached data as part of IXCLRDataProcess::Flush (dotnet#110700)
Found this while running the diagnostics repo SOS tests with the cDAC enabled. General sequence for the repro was: ``` !sethostruntime -none !bpmd <firstLocation> !bpmd <secondLocation> g g !clrstack ``` Printed stack shows `<unknown>` for some method(s). Between the first and second breakpoints, more methods were jitted and the corresponding code heap list updated. When a new method in the stack for `<secondLocation>` had the same code heap list as any method from `<firstLocation>`, we'd end up with a stale end address for the heap list and determine that the method was invalid (outside the address range). The cdac was assuming that the target would be created every time the state changes, but that is not the case (for the repro above, `!sethostruntime -none` resulted in not re-creating the target). We need to handle `IXCLRDataProcess::Flush` calls to clear out any cached data. With this change, the SOS tests with the cDAC enabled run successfully with both a Release and Debug cdacreader (on a Release runtime).
1 parent 1502947 commit fd3c397

File tree

4 files changed

+28
-10
lines changed

4 files changed

+28
-10
lines changed

src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ public interface IDataCache
139139
/// <param name="data">On return, set to the cached data value, or null if the data hasn't been cached yet.</param>
140140
/// <returns>True if a copy of the data is cached, or false otherwise</returns>
141141
bool TryGet<T>(ulong address, [NotNullWhen(true)] out T? data);
142+
/// <summary>
143+
/// Clear all cached data
144+
/// </summary>
145+
void Clear();
142146
}
143147

144148
/// <summary>

src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,11 @@ public bool TryGet<T>(ulong address, [NotNullWhen(true)] out T? data)
519519

520520
return false;
521521
}
522+
523+
public void Clear()
524+
{
525+
_readDataByAddress.Clear();
526+
}
522527
}
523528

524529
private readonly struct Reader(ReadFromTargetDelegate readFromTarget)

src/native/managed/cdacreader/src/Legacy/SOSDacImpl.IXCLRDataProcess.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,15 @@ namespace Microsoft.Diagnostics.DataContractReader.Legacy;
1414
internal sealed unsafe partial class SOSDacImpl : IXCLRDataProcess, IXCLRDataProcess2
1515
{
1616
int IXCLRDataProcess.Flush()
17-
=> _legacyProcess is not null ? _legacyProcess.Flush() : HResults.E_NOTIMPL;
17+
{
18+
_target.ProcessedData.Clear();
19+
20+
// As long as any part of cDAC falls back to the legacy DAC, we need to propagate the Flush call
21+
if (_legacyProcess is not null)
22+
return _legacyProcess.Flush();
23+
24+
return HResults.S_OK;
25+
}
1826

1927
int IXCLRDataProcess.StartEnumTasks(ulong* handle)
2028
=> _legacyProcess is not null ? _legacyProcess.StartEnumTasks(handle) : HResults.E_NOTIMPL;

src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -230,19 +230,17 @@ public override Target.TypeInfo GetTypeInfo(DataType dataType)
230230
public override ContractRegistry Contracts => _contractRegistry;
231231

232232
// A data cache that stores data in a dictionary and calls IData.Create to construct the data.
233-
private class DefaultDataCache : Target.IDataCache
233+
private sealed class DefaultDataCache : Target.IDataCache
234234
{
235-
protected readonly Target _target;
236-
protected readonly Dictionary<(ulong, Type), object?> _readDataByAddress = [];
235+
private readonly Target _target;
236+
private readonly Dictionary<(ulong, Type), object?> _readDataByAddress = [];
237237

238238
public DefaultDataCache(Target target)
239239
{
240240
_target = target;
241241
}
242242

243-
public virtual T GetOrAdd<T>(TargetPointer address) where T : Data.IData<T> => DefaultGetOrAdd<T>(address);
244-
245-
protected T DefaultGetOrAdd<T>(TargetPointer address) where T : Data.IData<T>
243+
public T GetOrAdd<T>(TargetPointer address) where T : Data.IData<T>
246244
{
247245
if (TryGet(address, out T? result))
248246
return result;
@@ -258,9 +256,7 @@ protected T DefaultGetOrAdd<T>(TargetPointer address) where T : Data.IData<T>
258256
return result!;
259257
}
260258

261-
public virtual bool TryGet<T>(ulong address, [NotNullWhen(true)] out T? data) => DefaultTryGet<T>(address, out data);
262-
263-
protected bool DefaultTryGet<T>(ulong address, [NotNullWhen(true)] out T? data)
259+
public bool TryGet<T>(ulong address, [NotNullWhen(true)] out T? data)
264260
{
265261
data = default;
266262
if (!_readDataByAddress.TryGetValue((address, typeof(T)), out object? dataObj))
@@ -273,6 +269,11 @@ protected bool DefaultTryGet<T>(ulong address, [NotNullWhen(true)] out T? data)
273269
}
274270
return false;
275271
}
272+
273+
public void Clear()
274+
{
275+
_readDataByAddress.Clear();
276+
}
276277
}
277278

278279
}

0 commit comments

Comments
 (0)