microsoft / azure-devops-dotnet-samples

.NET/C# samples for integrating with Azure DevOps Services and Azure DevOps Server
https://docs.microsoft.com/azure/devops/integrate
MIT License
523 stars 514 forks source link

VssRequestTimerTrace P/Invokes into ADVAPI32.DLL #35

Open abock opened 7 years ago

abock commented 7 years ago

Not sure if this is the best place to report an issue in the base VSTS .NET libraries, but here goes!

We're starting to use the .NET VSTS API at Xamarin for cross-CI integration on macOS and immediately ran into an issue where anything using Microsoft.VisualStudio.Services.WebApi.VssHttpClientBase (e.g. everything) results in an exception when run on macOS (Mono):

[ERROR] FATAL UNHANDLED EXCEPTION: System.DllNotFoundException: ADVAPI32.DLL
  at (wrapper managed-to-native) Microsoft.VisualStudio.Services.WebApi.VssRequestTimerTrace:EventActivityIdControl (int,System.Guid&)
  at Microsoft.VisualStudio.Services.WebApi.VssRequestTimerTrace.TraceRequest (System.Net.Http.HttpRequestMessage message) [0x00028] in <2918af0a6c8945db9347a85022c96b21>:0
  at Microsoft.VisualStudio.Services.WebApi.HttpMessageExtensions.Trace (System.Net.Http.HttpRequestMessage request) [0x0003b] in <2918af0a6c8945db9347a85022c96b21>:0
  at Microsoft.VisualStudio.Services.WebApi.VssHttpClientBase+<SendAsync>d__46.MoveNext () [0x000dd] in <2918af0a6c8945db9347a85022c96b21>:0

It would be great if this call could be avoided on non-Windows systems as it is not necessary to the functionality of the .NET VSTS API at all.

The call is made at the end of VssRequestTimerTrace.TraceRequest (HttpRequestMessage). It'd be great to just guard it with a check against Environment.OSVersion.Platform or similar (e.g. on macOS this will be PlatformID.Unix on Mono and possibly PlatformID.MacOSX on another runtime (.NET Core?)).

I'd probably also guard the call itself as well by catching DllNotFoundException, however an explicit platform check will be much faster than performing the P/Invoke and catching the failure (Mono will attempt to resolve many different patterns of native library names before giving up).

Workaround

As a workaround, I mocked a libADVAPI32.DLL.dylib to ensure the P/Invoke succeeds:

ADVAPI32.c:

unsigned int
EventActivityIdControl (int ControlCode, void *ActivityId)
{
    return 0;
}

Compile:

clang -o libADVAPI32.DLL.dylib ADVAPI32.c -dynamiclib -arch x86_64 -arch i386

Run my test tool:

Build:
  id: 847943
  url: https://abc.visualstudio.com/guid/_apis/build/Builds/847943
  status: NotStarted
  status: InProgress....................................................................................................................................................
  status: Completed
  result: Succeeded
  duration: 00:12:32.0572594

In other words, if we can just avoid this P/Invoke on non-Windows systems, the libraries work (for at least our current use case: build queueing and monitoring) just fine!

trydis commented 7 years ago

Thanks for the workaround, ran head first into this one on OS X/Mono.

wgv-srbrills commented 5 years ago

We also ran into this issue on 12/12/2018, using the latest versions. Still a blocker on VS for Mac.

tedchamb commented 5 years ago

Our preview NuGet packages contain NetStandard versions of our libraries. The call from the stack trace above to EventActivityIdControl does not exist in our NetStandard version of this assembly. The net45 version of the binary is meant for windows only, so use the NetStandard version for all other operating systems.