search

dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
671 stars 349 forks source link

Update CLRMD in RemoteExecutor #8483

Open danmoseley opened 2 years ago

danmoseley commented 2 years ago

This utility, used by the libraries tests, uses clrmd for creating dumps and logging info out of them when a (child) process hangs:

https://github.com/dotnet/arcade/blob/49750c02e63d0ad3a77d035bba7498a0b1acd218/src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs#L178-L211

It's using Microsoft.Diagnostics.Runtime 1.0.5 - I went to update it to the latest but I see a bunch of API has changed/gone.

@leculver I see you did a lot of work for 2.0.x -- are there improvements in 2.0.x that might be worth us updating this? Note that it’s wired up to only work on Windows right now – it would be interesting to have it work on Unix.

Otherwise I’ll leave as is.

cc @stephentoub who added it

leculver commented 2 years ago

ClrMD 2.0 does work on Unix (both linux and os x), and overall it's a much better product. I think some of those specific apis (IsThreadpool*) are missing, but IIRC that's just an oversight. If you decide to do an update of the tool I can add those APIs to 2.0 and ship a new build. There's already an issue for those missing apis...

danmoseley commented 2 years ago

@leculver it is working fine, so low priority, but if and when you do plug those gaps let me know and I'll make the update.

JimBobSquarePants commented 1 year ago

Just to note. The version of Microsoft.Diagnostics.Runtime currently used makes it impossible to use RemoteExecutor in a project referencing BenchmarkDotNet 13.0.10 due to API changes. 

System.MissingMethodException : Method not found: 'Microsoft.Diagnostics.Runtime.DataTarget Microsoft.Diagnostics.Runtime.DataTarget.AttachToProcess(Int32, UInt32)'.
leculver commented 1 year ago

Ah I forgot about this issue. It wasn't assigned to me so it wasn't on my radar. The missing pieces should be there in ClrMD 3.1. They are just under the thread's status. If I remember, I will take a look at this when I'm back from vacation on the 20th.

JimBobSquarePants commented 12 months ago

That would be awesome!

JimBobSquarePants commented 2 weeks ago

@leculver Was just about to update BenchmarkDotNet due to vulnerabilities in System.Text.RegularExpressions but realised this may still be a factor. Was this ever updated?