Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@
<MicrosoftBuildUtilitiesCoreVersion>$(MicrosoftBuildVersion)</MicrosoftBuildUtilitiesCoreVersion>
<NugetProjectModelVersion>6.2.2</NugetProjectModelVersion>
<NugetPackagingVersion>6.2.2</NugetPackagingVersion>
<DotnetSosVersion>7.0.412701</DotnetSosVersion>
<DotnetSosTargetFrameworkVersion>6.0</DotnetSosTargetFrameworkVersion>
<!-- Testing -->
<MicrosoftNETCoreCoreDisToolsVersion>1.1.0</MicrosoftNETCoreCoreDisToolsVersion>
<MicrosoftNETTestSdkVersion>17.4.0-preview-20220707-01</MicrosoftNETTestSdkVersion>
Expand Down
7 changes: 5 additions & 2 deletions src/tests/Common/Coreclr.TestWrapper/CoreclrTestWrapperLib.cs
Original file line number Diff line number Diff line change
Expand Up @@ -601,10 +601,13 @@ static bool TryPrintStackTraceFromDmp(string dmpFile, StreamWriter outputWriter)
return false;
}

string sosPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".dotnet", "sos", "sos.dll");

var cdbScriptPath = Path.GetTempFileName();
// TODO: Add SOS support once we can easily download SOS to install.
File.WriteAllText(cdbScriptPath, """
File.WriteAllText(cdbScriptPath, $$"""
Comment thread
jkoritzinsky marked this conversation as resolved.
Outdated
.load {{sosPath}}
~*k
!clrstack -f -all
q
""");

Expand Down
10 changes: 10 additions & 0 deletions src/tests/Common/helixpublishwitharcade.proj
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,10 @@

<!-- Browser-Wasm follows a very different workflow, which is currently out of scope of the Log Checker. -->
<HelixCorrelationPayload Include="$(XUnitLogCheckerDirectory)" Condition="'$(TargetsBrowser)' != 'true'" />
<HelixCorrelationPayload Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="dotnet-sos">
<Destination>sos</Destination>
<Uri>https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/flat2/dotnet-sos/$(DotnetSosVersion)/dotnet-sos.$(DotnetSosVersion).nupkg</Uri>
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.

Before dotnet-sos existed, we used clrmd directly in RemoteExecutor:
https://github.com/dotnet/arcade/blob/bdc59254cf108e1d48451dc43bb9ebc331cdca7b/src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs#L177-L225
We might want to standardize on one or the other.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ClrMD there is doing process attach only for timeouts, whereas we're handling both timeouts and crashes here, so it's a little different. Definitely worth considering consolidation though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There were some talks about standardizing this sort of mechanism (dumping stack traces from a dump to stdout for the build analysis tooling), but with the recent changes to the EngSrv teams, I don't know how much of that work will still happen.

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.

Right, I understand they're handling different cases. But they can both handle both, so it's overkill to have two different tools used for the same purpose. Simply suggesting we choose one and stick with it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll look at the RemoteExecutor implementation and see if we can consolidate something useful.

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.

Cool, thanks.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We'll need to use something like this pattern to get both native and managed stack traces, and we'll likely want to use this model as well to solve #83047 (which focuses more on crashes than timeouts) if we don't move to using dotnet test for the libraries tests in Helix. The RemoteExecutor version has a much better UX around the output though as it has more structure to work with (from CLRMD's APIs).

</HelixCorrelationPayload>

<LegacyPayloads Include="$([System.IO.Directory]::GetDirectories($(LegacyPayloadsRootDirectory)))" Condition="Exists('$(LegacyPayloadsRootDirectory)')" />
<LegacyPayloads Update="@(LegacyPayloads)">
Expand Down Expand Up @@ -867,4 +871,10 @@
<Import Sdk="Microsoft.DotNet.Helix.Sdk" Project="Sdk.targets" Condition=" '$(UsesHelixSdk)' == 'true' " />
<Import Sdk="Microsoft.Build.NoTargets" Project="Sdk.targets" Condition=" '$(UsesHelixSdk)' != 'true' " />

<!-- This target needs to come after importing the Helix SDK as AfterTargets doesn't work for targets that have yet to be defined. -->
<Target Name="ConfigureSOS" AfterTargets="AddDotNetSdk" BeforeTargets="CoreTest" Condition="'$(TestWrapperTargetsWindows)' == 'true' and '$(RuntimeFlavor)' == 'CoreCLR'">
<PropertyGroup>
<HelixPreCommands>$(HelixPreCommands);dotnet %25HELIX_CORRELATION_PAYLOAD%25\sos\tools\net$(DotnetSosTargetFrameworkVersion)\any\dotnet-sos.dll install --architecture $(TargetArchitecture)</HelixPreCommands>
</PropertyGroup>
</Target>
</Project>