Basic implementation for testing of COM activation of a .NET class #19760

AaronRobinsonMSFT · 2018-08-30T04:26:58Z

This represents a partial implementation for dotnet/core-setup#4476

It current contains the following:

minimal SPCL API implementation - only for RegFree COM scenario
implementation of a host library for testing of activation scenarios
- Note the CoreShim library is analogous to the CoreRun exe and only for testing
basic unit test of the SPCL API
simple e2e test using RegFree COM

AaronRobinsonMSFT · 2018-08-31T01:36:55Z

src/coreclr/hosts/coreshim/CoreShim.cpp

@@ -0,0 +1,275 @@
+// Licensed to the .NET Foundation under one or more agreements.


cc @morganbr This is the library I am adding to enable hosting when starting in a native entry point. I have also written a mechanism that allows native EXEs to be launched as the test entry point.

Is this library a shipping code, or just a test-only code (like corerun.exe)?

This is equivalent to corerun.exe - my understanding is that all the projects under the hosts/ are for testing and all real hosts should be contained in the core-setup repo. Am I missing the point of this directory?

AaronRobinsonMSFT · 2018-08-31T01:43:04Z

Will be filling out the Native client to match the support already present in the .NET client. Once this is complete, the basics for managed activation will be defined. Additional investigations will can then be used to address the gaps in the proposal dotnet/core-setup#4476.

AaronRobinsonMSFT · 2018-08-31T02:22:30Z

@dotnet-bot test all please

AaronRobinsonMSFT · 2018-08-31T05:12:37Z

tests/src/Interop/common/ExeLauncherProgram.cs

+        Console.WriteLine($"Searching for exe to launch in {workingDir}...");
+
+        string startExe = string.Empty;
+        foreach (string exeMaybe in Directory.EnumerateFiles(workingDir, "*.exe"))


Need to filter out the launch module itself.

jkotas · 2018-08-31T20:06:55Z

src/System.Private.CoreLib/src/System/Runtime/InteropServices/ComActivator.cs

+                        assemPathLocal = Path.ChangeExtension(assemPath, ".dll");
+                    }
+
+                    assem = Assembly.LoadFrom(assemPathLocal);


Loading a bunch of assemblies using LoadFrom is asking for problems. The dependencies of these assemblies may not be resolved correctly, and the app may not expect all of them to be loaded.

And then loading all types in these assemblies using GetTypes is performance trap.

Is this temporary hack?

Yes. This is purely for satisfying test bring up. Once the API surface area is signed off, we need to do a lot more here. Figure out ALC and such along with probably trying to parse metadata instead of loading the assembly itself. Perhaps using Assembly.ReflectionOnlyLoadFrom() in the long run?

I will be looking to @vitek-karas to keep my honest here :-) Is the Assembly.ReflectionOnlyLoadFrom() the right API to interrogate the types in an assembly without perturbing the running CLR instance?

ReflectionOnlyLoadFrom is not supported in .NET Core.

Is there no way to avoid the linear search to find the right type? I think we should have the type name in some manifest to avoid the linear search.

Is there no way to avoid the linear search to find the right type? I think we should have the type name in some manifest to avoid the linear search.

There are options I still need to explore, but in this case that is really secondary. The real issue is finding which assembly in the activation context is the assembly with the type in question. In the example there is one assembly, but conceivably there could be N assemblies and each need to be queried to determine the one that provides the type.

I guess the better question here is what is the appropriate way to read an assemblies metadata without loading the assembly into the current CLR?

Unmanaged: IMDInternalImport interface

Managed: System.Reflection.Metadata https://github.com/dotnet/corefx/tree/master/src/System.Reflection.Metadata

AaronRobinsonMSFT · 2018-09-03T23:13:04Z

cc @vitek-karas This PR represents the broad strokes of the design described in dotnet/core-setup#4476

vitek-karas · 2018-09-04T11:44:29Z

src/System.Private.CoreLib/src/System/Runtime/InteropServices/ComActivator.cs

+        {
+            private readonly Guid classId;
+            private readonly Type classType;
+            private readonly Assembly classAssembly;


There's no need to store the assembly here - if nothing else it can be asked on the type, but the class should not need to know the assembly to do anything anyway.

AaronRobinsonMSFT · 2018-09-05T21:41:30Z

@dotnet-bot test all please

AaronRobinsonMSFT · 2018-09-05T21:41:51Z

@dotnet-bot help

dotnet-bot · 2018-09-05T21:41:55Z

Welcome to the dotnet/coreclr Repository

The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR

The following commands are valid for all PRs and repositories.

Click to expand

Comment Phrase	Action
@dotnet-bot test this please	Re-run all legs. Use sparingly
@dotnet-bot test ci please	Generates (but does not run) jobs based on changes to the groovy job definitions in this branch
@dotnet-bot help	Print this help message

The following jobs are launched by default for each PR against dotnet/coreclr:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test Linux-musl x64 Debug Build	Linux-musl x64 Debug Build

The following optional jobs are available in PRs against dotnet/coreclr:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test Outerloop Linux-musl x64 Debug Build	Queues Outerloop Linux-musl x64 Debug Build
@dotnet-bot test Linux-musl x64 Release Build	Queues Linux-musl x64 Release Build
@dotnet-bot test Outerloop Linux-musl x64 Release Build	Queues Outerloop Linux-musl x64 Release Build

Have a nice day!

AaronRobinsonMSFT · 2018-09-05T21:42:24Z

@dotnet-bot test ci please

dotnet-bot · 2018-09-05T21:42:56Z

Welcome to the dotnet/coreclr Perf help

The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR

The following commands are valid for all PRs and repositories.

Click to expand

Comment Phrase	Action
@dotnet-bot test this please	Re-run all legs. Use sparingly
@dotnet-bot test ci please	Generates (but does not run) jobs based on changes to the groovy job definitions in this branch
@dotnet-bot help	Print this help message

The following jobs are launched by default for each PR against dotnet/coreclr:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test \QWindows_NT x64 full_opt ryujit CoreCLR Perf Tests Correctness\E	Windows_NT x64 full_opt ryujit CoreCLR Perf Tests Correctness
@dotnet-bot test \QWindows_NT x64 min_opt ryujit CoreCLR Perf Tests Correctness\E	Windows_NT x64 min_opt ryujit CoreCLR Perf Tests Correctness
@dotnet-bot test \QWindows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness\E	Windows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness
@dotnet-bot test \QWindows_NT x86 min_opt ryujit CoreCLR Perf Tests Correctness\E	Windows_NT x86 min_opt ryujit CoreCLR Perf Tests Correctness

The following optional jobs are available in PRs against dotnet/coreclr:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test Windows_NT x64 illink	Queues Windows_NT x64 full_opt ryujit IlLink Tests
@dotnet-bot test linux perf flow	Queues Linux Perf Test Flow
@dotnet-bot test Windows_NT x64 perf	Queues Windows_NT x64 full_opt ryujit CoreCLR Perf Tests
@dotnet-bot test Windows_NT x64 min_opts perf	Queues Windows_NT x64 min_opt ryujit CoreCLR Perf Tests
@dotnet-bot test Windows_NT x86 perf	Queues Windows_NT x86 full_opt ryujit CoreCLR Perf Tests
@dotnet-bot test Windows_NT x86 min_opts perf	Queues Windows_NT x86 min_opt ryujit CoreCLR Perf Tests
@dotnet-bot test Windows_NT x64 perf scenarios	Queues Windows_NT x64 full_opt ryujit Performance Scenarios Tests
@dotnet-bot test Windows_NT x64 min_opts perf scenarios	Queues Windows_NT x64 min_opt ryujit Performance Scenarios Tests
@dotnet-bot test Windows_NT x64 perf scenarios	Queues Windows_NT x64 tiered ryujit Performance Scenarios Tests
@dotnet-bot test Windows_NT x86 perf scenarios	Queues Windows_NT x86 full_opt ryujit Performance Scenarios Tests
@dotnet-bot test Windows_NT x86 min_opts perf scenarios	Queues Windows_NT x86 min_opt ryujit Performance Scenarios Tests
@dotnet-bot test Windows_NT x86 perf scenarios	Queues Windows_NT x86 tiered ryujit Performance Scenarios Tests
@dotnet-bot test linux throughput flow	Queues Linux arm Throughput Perf Test Flow
@dotnet-bot test linux throughput flow	Queues Linux x64 Throughput Perf Test Flow
@dotnet-bot test Windows_NT x64 throughput	Queues Windows_NT x64 full_opt ryujit nopgo CoreCLR Throughput Perf Tests
@dotnet-bot test Windows_NT x64 nopgo throughput	Queues Windows_NT x64 full_opt ryujit pgo CoreCLR Throughput Perf Tests
@dotnet-bot test Windows_NT x64 min_opts throughput	Queues Windows_NT x64 min_opt ryujit nopgo CoreCLR Throughput Perf Tests
@dotnet-bot test Windows_NT x64 min_opts nopgo throughput	Queues Windows_NT x64 min_opt ryujit pgo CoreCLR Throughput Perf Tests
@dotnet-bot test Windows_NT x86 throughput	Queues Windows_NT x86 full_opt ryujit nopgo CoreCLR Throughput Perf Tests
@dotnet-bot test Windows_NT x86 nopgo throughput	Queues Windows_NT x86 full_opt ryujit pgo CoreCLR Throughput Perf Tests
@dotnet-bot test Windows_NT x86 min_opts throughput	Queues Windows_NT x86 min_opt ryujit nopgo CoreCLR Throughput Perf Tests
@dotnet-bot test Windows_NT x86 min_opts nopgo throughput	Queues Windows_NT x86 min_opt ryujit pgo CoreCLR Throughput Perf Tests

Have a nice day!

AaronRobinsonMSFT · 2018-09-05T21:43:48Z

@dotnet-bot test this please

AaronRobinsonMSFT · 2018-09-05T23:58:14Z

@jkotas @luqunl @vitek-karas @jeffschwMSFT Any additional concerns/comments on this?

morganbr · 2018-09-06T01:19:28Z

src/coreclr/hosts/coreshim/CoreShim.cpp

+        std::wstring pathLocal;
+        if (path == nullptr)
+        {
+            pathLocal = GetEnvVar(W("CORE_ROOT"));


Is this temporary? If you intend for it to be permanent, we should talk about it.

This is what we want for testing because that is where coreclr is consumed during testing. What are you thinking it should be?

If this is test code only, no concerns.

morganbr · 2018-09-06T01:24:23Z

src/coreclr/hosts/coreshim/CoreShim.cpp

+
+        pathLocal.append(W("\\coreclr.dll"));
+
+        AutoModule hmod = ::LoadLibraryExW(pathLocal.c_str() , nullptr, LOAD_WITH_ALTERED_SEARCH_PATH);


We don't want the full behavior of altered_search_path, we just want to search the directory we asked for. I think that would be LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR. Note that using that flag requires first probing for the AddDllDirectory API similar to:

coreclr/src/vm/dllimport.cpp

Lines 6524 to 6532 in 246ae78

// Check if the OS supports the new secure LoadLibraryEx flags introduced in KB2533623

HMODULE hMod = CLRGetModuleHandle(WINDOWS_KERNEL32_DLLNAME_W);

_ASSERTE(hMod != NULL);

if (GetProcAddress(hMod, "AddDllDirectory") != NULL)

{

// The AddDllDirectory export was added in KB2533623 together with the new flag support

s_fSecureLoadLibrarySupported = true;

}

I can change to that, but I want to be clear here this shim is only for the testing environment. There are way bigger issues in here if this was going to be used as the actual product shim.

If this is just testing, I'm not concerned. You might still want LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR to avoid accidentally picking up stray CoreCLRs from your path or working directory though.

That is a good suggestion and i will update it. Thanks.

morganbr · 2018-09-06T01:25:38Z

src/coreclr/hosts/coreshim/CoreShim.cpp

+        std::wstring w_dirLocal;
+        if (dir == nullptr)
+        {
+            w_dirLocal = GetEnvVar(W("CORE_ROOT"));


Same question about this variable.

vitek-karas · 2018-09-06T08:58:08Z

The test hosts are fine (test code for now, so no trouble). But the managed part in System.Private.* should be product quality. Currently there are several todos there:

Finding the type by GUID - loading the assemblies, linear search and so on.
Logging
Registry interaction

I think we should have at least understanding what are the plans on these.

AaronRobinsonMSFT · 2018-09-06T16:25:37Z

Finding the type by GUID - loading the assemblies, linear search and so on.

This is going to be done through metadata, but at the moment isn't needed for testing. There are several outstanding questions here, but the API contract is sound and is basically the point of the PR at present. A side note here, is that we have a basic trade-off here. Do a linear search, create yet another parser in native code (i.e. XML), or move activation into an external NuGet library. All are valid options, but have significantly different costs. At present the linear search is enough for testing and isn't exposed and can be improved upon behind the present interface.

Logging

I have stubs for logging and will fill them in as soon as I hear of an option. Rumor has it, you are going to be providing a design doc/proposal soon? 😄

Registry interaction

I will add a stub for this. My plan is to handle all registry access in native instead of relying on the managed API. The stub I will add will be a QCall into coreclr.dll and it will return the registry details. The intent of doing the work in native is that it will keep registry access in a location that is quick to implement and avoid creating additional managed library dependencies.

jkotas · 2018-09-06T16:49:45Z

it will keep registry access in a location that is quick to implement and avoid creating additional managed library dependencies.

The managed registry APIs are available in CoreLib. (I do not have a problem with doing this via QCall ... but just want to make sure you will do it for the right reason.)

jkotas · 2018-09-06T16:52:07Z

src/System.Private.CoreLib/src/System/Runtime/InteropServices/ComActivator.cs

+        private static bool IsLoggingEnabled()
+        {
+#if DEBUG
+            return true;


We should not be doing this logging by default to debug output in debug builds. It is annoying to keep looking at somebody else's logs in debug output that you do not care about.

Sounds good. I think the enable/disable logic is something I need guidance on, but is really apart of the general logging/diagnostic approach. @vitek-karas any thoughts on how we would like to enable disable this?

If it's only for development of this thing for now, then a compile time define would be enough.
If we think we need this a bit broadly... the answer depends on the discussion about logging below. If we're going to use the same infra as binder, then you'll get it for free.
If not, then you could use EventSource directly.

AaronRobinsonMSFT · 2018-09-06T16:58:49Z

The managed registry APIs are available in CoreLib.

@jkotas w00t! Thanks for letting me know. I was just trying to avoid bringing in more managed dependencies, but if this already exists, I will investigate this approach. Another factor in my natice decision was to consolidate registry access logic. If I can remove the existing native registry logic, I will do it all in managed, otherwise back to QCall 😄

vitek-karas · 2018-09-06T17:06:32Z

@AaronRobinsonMSFT RE logging
I'll have to think about this - I didn't realize that there might be a connection between what I'm doing for the binder and this.
Really the only added value of the tracing infra I'm working on is related to the binder. So if this code doesn't call into the binder, or is not related to it in any way, there would be little value in using the same thing (it would probably just bring in confusion).

AaronRobinsonMSFT · 2018-09-07T16:20:42Z

@dotnet-bot test Ubuntu x64 Checked Innerloop Build and Test
@dotnet-bot test Ubuntu x64 Checked Innerloop Build and Test (Jit - TieredCompilation=0)

AaronRobinsonMSFT · 2018-09-07T22:27:14Z

@dotnet-bot test Ubuntu x64 Checked CoreFX Tests

…launch a local EXE. This is useful when the test scenario's entry point is native.

Add property to exclude default assertion file Display exe ExeLaunchProgram class is going to launch

…ontext

Consume the ExeLauncherProgram.cs file as a wrapper for the native test

…there is no way to determine that is the platform.

AaronRobinsonMSFT added area-Interop * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) labels Aug 30, 2018

AaronRobinsonMSFT added this to the 3.0 milestone Aug 30, 2018

AaronRobinsonMSFT force-pushed the add_managed_activation_spcl branch from b8c50af to b27c4db Compare August 31, 2018 01:33

AaronRobinsonMSFT commented Aug 31, 2018

View reviewed changes

AaronRobinsonMSFT added the os-windows label Aug 31, 2018

AaronRobinsonMSFT commented Aug 31, 2018

View reviewed changes

jkotas reviewed Aug 31, 2018

View reviewed changes

vitek-karas reviewed Sep 4, 2018

View reviewed changes

AaronRobinsonMSFT changed the title ~~[WIP] Rough outline of managed implementation for COM activation in SPCL~~ [WIP] Basic implementation for testing of COM activation of a .NET class Sep 5, 2018

AaronRobinsonMSFT changed the title ~~[WIP] Basic implementation for testing of COM activation of a .NET class~~ Basic implementation for testing of COM activation of a .NET class Sep 5, 2018

morganbr reviewed Sep 6, 2018

View reviewed changes

jkotas reviewed Sep 6, 2018

View reviewed changes

vitek-karas approved these changes Sep 7, 2018

View reviewed changes

AaronRobinsonMSFT removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 7, 2018

AaronRobinsonMSFT added 19 commits September 7, 2018 15:29

Rough outline of managed implementation for COM activation in SPCL

3db5f4b

Add logging surface

462326e

Add C# file that can be used as a managed entry point for testing to …

0fc04b7

…launch a local EXE. This is useful when the test scenario's entry point is native.

Add property for finding interop common

ba3c980

Add property to exclude default assertion file Display exe ExeLaunchProgram class is going to launch

Display command running in build-test.cmd

31e2deb

Add a new entry point for managed COM activation from a native caller.

bbad31b

Add a new host entry point for CLR activation from a loaded library c…

be666c7

…ontext

Add support for copying the CoreShim library to local test directory

29dac9a

Implement ErrorMarshalTesting class in C#

bda8370

Add a native client for the NETServer

4b88a15

Consume the ExeLauncherProgram.cs file as a wrapper for the native test

Block ExeLauncherProgram from recursively launching itself.

05dc0a8

Update COM Server contracts to use 'int' instead of 'long'

350e120

Fix managed marshalling contract for IArrayTesting interface

7fb7ace

Complete symmetric testing coverage for .NET server and native client.

898c16a

Remove explicit storage of associated Assembly for class Type.

f4f5085

Block EXE launch from running on non-Windows machines

1f0c0ba

Mark main entry point as __cdecl for x86 build

60b0f77

Address feedback

0f14fa6

Disable COM testing in helix since it has issues on Windows Nano and …

b607d44

…there is no way to determine that is the platform.

AaronRobinsonMSFT force-pushed the add_managed_activation_spcl branch from 531d058 to b607d44 Compare September 7, 2018 22:32

AaronRobinsonMSFT added 2 commits September 10, 2018 15:14

Update tests based on upcoming clsid mapping manifest approach.

b16531b

Add missing header file

9c56d6d

AaronRobinsonMSFT merged commit fc33780 into dotnet:master Sep 11, 2018

AaronRobinsonMSFT deleted the add_managed_activation_spcl branch September 11, 2018 00:26

ghost mentioned this pull request Jan 31, 2020

New COM Activation tests broke official test build dotnet/runtime#11065

Closed

sandreenko mentioned this pull request Jan 31, 2020

Interop\COM\NativeClients\Primitives do not see files in Core_Root dotnet/runtime#11556

Closed

		@@ -0,0 +1,275 @@
		// Licensed to the .NET Foundation under one or more agreements.


		pathLocal.append(W("\\coreclr.dll"));

		AutoModule hmod = ::LoadLibraryExW(pathLocal.c_str() , nullptr, LOAD_WITH_ALTERED_SEARCH_PATH);

	// Check if the OS supports the new secure LoadLibraryEx flags introduced in KB2533623
	HMODULE hMod = CLRGetModuleHandle(WINDOWS_KERNEL32_DLLNAME_W);
	_ASSERTE(hMod != NULL);

	if (GetProcAddress(hMod, "AddDllDirectory") != NULL)
	{
	// The AddDllDirectory export was added in KB2533623 together with the new flag support
	s_fSecureLoadLibrarySupported = true;
	}

Basic implementation for testing of COM activation of a .NET class #19760

Basic implementation for testing of COM activation of a .NET class #19760

Conversation

AaronRobinsonMSFT commented Aug 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronRobinsonMSFT commented Aug 31, 2018

AaronRobinsonMSFT commented Aug 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronRobinsonMSFT commented Sep 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronRobinsonMSFT commented Sep 5, 2018

AaronRobinsonMSFT commented Sep 5, 2018

dotnet-bot commented Sep 5, 2018

AaronRobinsonMSFT commented Sep 5, 2018

dotnet-bot commented Sep 5, 2018

AaronRobinsonMSFT commented Sep 5, 2018

AaronRobinsonMSFT commented Sep 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitek-karas commented Sep 6, 2018

AaronRobinsonMSFT commented Sep 6, 2018 • edited Loading

jkotas commented Sep 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronRobinsonMSFT commented Sep 6, 2018

vitek-karas commented Sep 6, 2018

AaronRobinsonMSFT commented Sep 7, 2018

AaronRobinsonMSFT commented Sep 7, 2018

AaronRobinsonMSFT commented Aug 30, 2018 •

edited

Loading

AaronRobinsonMSFT commented Sep 5, 2018 •

edited

Loading

AaronRobinsonMSFT commented Sep 6, 2018 •

edited

Loading

jkotas commented Sep 6, 2018 •

edited

Loading