Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Add System.Management #24719

Merged
merged 30 commits into from
Oct 25, 2017
Merged

Add System.Management #24719

merged 30 commits into from
Oct 25, 2017

Conversation

pjanotti
Copy link
Contributor

@pjanotti pjanotti commented Oct 18, 2017

Bringing System.Management, a.k.a WMI, as a CoreFX package that can run on Windows.

I am hitting issues in Unix with generated assemblies: CA1821 empty finalizers, I will investigate the options here, it can't be pushed as it is.

I still have to validate build on uap and uapaot.

After this initial commit more tests and code clean up and formatting are still going to be added/done the main focus here is to get projects/build correctly set up.

Fixes #14762

The finalizers are being generated empty for PNSE assembly. In such
cases disable CA1821.
@tarekgh
Copy link
Member

tarekgh commented Oct 18, 2017

I left some comments mostly about the code formatting. did we run the formatter tool on the introduced source files?

@pjanotti
Copy link
Contributor Author

No, I didn't run the code formatter. There was some discussion to not run it against this code but after first commit do the VS ones (e.g.: rename fields, etc).

@pjanotti
Copy link
Contributor Author

My main concern at this moment is with the projects/build related stuff.

@tarekgh
Copy link
Member

tarekgh commented Oct 18, 2017

but after first commit do the VS ones (e.g.: rename fields, etc).

We should be careful renaming the fields as I am seeing the classes support serialization. So if we need to be compatible with the desktop serialization, we’ll need to keep the same field names.

@pjanotti
Copy link
Contributor Author

Thanks @tarekgh! I wasn't thinking about serialization, will have to look before doing anything.

}
Copy link
Member

Choose a reason for hiding this comment

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

NIT: we can undo this change 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was automatically made by some tool 😄 I will undo it.

<SupportedFramework>netcoreapp2.0</SupportedFramework>
</ProjectReference>
<ProjectReference Include="..\src\System.Management.csproj" />
<InboxOnTargetFramework Include="net45">
Copy link
Member

Choose a reason for hiding this comment

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

I would have expected the package index to also contain a change about this new library and where is it inboxed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ops forgot that, also what is the correct version? the smallest possible?

Copy link
Member

Choose a reason for hiding this comment

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

do you mean the assembly version? If so, we should use whatever is on desktop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was already an entry for System.Management in the packages index, but now with the other changes of build configurations, I've been unable to get a clean local build (yet).

@pjanotti
Copy link
Contributor Author

@danmosemsft @joperezr per conversation yesterday with @weshaggard this one should not have a facade, since it has a module with same name on desktop, so I went back to initial format. Please, take a look at the current changes (Linux failures are unrelated).

@pjanotti pjanotti requested a review from weshaggard October 21, 2017 15:40
@4creators
Copy link
Contributor

This PR is related to API discussion from issue https://github.com/dotnet/corefx/issues/22660 Since Linux is slowly growing support for CIM and WMI infrastructure it would be very interesting to exploit possibilities which this creates for support of System.Management on Linux. See Windows Management Instrumentation Now A Formal Bus With Linux 4.13

@@ -3,7 +3,7 @@
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<ItemGroup>
<ProjectReference Include="..\ref\System.Management.csproj">
<SupportedFramework>netcoreapp2.0;net45;$(AllXamarinFrameworks)</SupportedFramework>
<SupportedFramework>netcoreapp2.0;net45;$(UAPvNextTFM);$(AllXamarinFrameworks)</SupportedFramework>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this in other libraries? Only 5 libraries already have it.

And System.Drawing.Common.pkproj eg doesn't have net45/net461 either:
<SupportedFramework>netcoreapp2.0</SupportedFramework>

@safern I wonder if all these need regularizing as well.

Copy link
Member

@safern safern Oct 25, 2017

Choose a reason for hiding this comment

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

System.Drawing.Common.pkproj eg doesn't have net45/net461 either.

I'm adding net461 into System.Drawing.Common and also AllXamarinFrameworks in a PR I have open.

UAPvNextTFM I don't think we need it since it will already be netstandard.

Yes, this need regularizing as well from the quick look I just took. I can work with @pjanotti offline to get it according to what we have in the other projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per my conversation with Tarek that is my understanding: this needs regularizing.

UAPvNextTFM was added at this moment while adding the PNS assembly for uap, otherwise we hit: "System.Management should not be supported on UAP,Version=v10.0.15138/win10-x86 but has runtime assets "

Alternatively we can keep it closer to System.DirectoryServices.AccountManagement that doesn't have a PNS assembly for uap yet.

That said I want to make this initial commit soon so I can focus more on the test coverage and the sources themselves, so far I've been most of the time dealing with build/packaging issues.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the UapVNextTFM is then needed. However we need netfx facades also. You can merge as is and I can revisit and fix the configurations if you want. But fixing them wouldn’t take us more than 1 hour.

Copy link
Member

Choose a reason for hiding this comment

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

You can take this PR as an example: #24758

@pjanotti pjanotti changed the title WIP: Add System.Management Add System.Management Oct 25, 2017
@pjanotti
Copy link
Contributor Author

I am merging this change so we get a separate commit for the "regularizing" step.

@pjanotti pjanotti merged commit 0f57714 into dotnet:master Oct 25, 2017
@karelz karelz added this to the 2.1.0 milestone Oct 28, 2017
@pjanotti pjanotti deleted the wmi branch October 30, 2017 16:59
pjanotti pushed a commit to pjanotti/corefx that referenced this pull request Oct 31, 2017
* First pass building

* Remove Instrumentation and WMIGenerator

* Resources to as used on CoreFX

Opportunistic: some small changes related to warnings

* Removed hard coded path to load wminet_utils

Also moved so DllImport to use Common version of files.

* Added ref, sln, and single basic test

Code builds and pass single test

* Add CoreFX source header

* Removing unused using directives

* clean src proj references

* A bit more test coverage (19%)

* Sanitizer pass with default settings

* Adding WMIGenerator

This allows methods to generate CodeDom for strong type access to WMI
objects.

* Missing files from previous commit

* Some extra clean-up missed by SSS

* Removing #ifdef dead code

* Add pkg project

* Removing more dead code

* Removing dead code and small improvement to tests

* Suppress CA1821 when generating PNSE assembly

The finalizers are being generated empty for PNSE assembly. In such
cases disable CA1821.

* Source changes from CR (so far)

* Targets and package issues

* Go back to project targets and settings

Per conversation w/ @weshaggard it should follow the same model as
System.DirectoryServices not the packages of dlls that do not exist on
desktop. This still is not expected to be fully correct but it puts the
projects closer to the targets that they will have in the end.

* Description for the System.Management package.

* Match supported pkg frameworks

* Skip tests on Windows Nano

* Missed files from previous commit

* Untabify files and a bit more tests

* Generate PNS Assembly for uap

* Add references needed by uap for PNS assembly

* PNS assembly message

* Fixing package issue
@jianxuanbing
Copy link

Hello,I have a questin.I use examples

ManagementClass diskClass = new ManagementClass("Win32_LogicalDisk");
diskClass.Get();
Output.WriteLine("Logical Disk class has " + diskClass.Properties.Count + " properties");

but is not ok,throw exception

System.TypeInitializationException : The type initializer for 'System.Management.WmiNetUtilsHelper' threw an exception.
---- System.NullReferenceException : Object reference not set to an instance of an object.
   at System.Management.MTAHelper.IsNoContextMTA()
   at System.Management.MTAHelper.CreateInMTA(Type type)
   at System.Management.ManagementPath.CreateWbemPath(String path)
   at System.Management.ManagementClass..ctor(String path)

@pjanotti
Copy link
Contributor Author

Hi, thanks for reporting it. I can't repro the issue. Please, provide the details and a small repro - feel free to open a separate issue with those details.

Notice that System.Management namespace is intended for legacy code only.

@jianxuanbing
Copy link

Hi,I changed the compter,the result is normal.
I will try again.

@jianxuanbing
Copy link

I use the same code. But at home when the computer tests is throw Exception.
But at company when the computer tests is ok.

        public static string GetName()
        {
            using (ManagementClass mc = new ManagementClass(WmiPathConst.Processor))
            {
                ManagementObjectCollection moc = mc.GetInstances();
                return moc.OfType<ManagementObject>()
                    .Select(mo => mo[ManagementObjectConst.Name].ToString())
                    .FirstOrDefault();
            }
        }

image
image

@STRATZ-Ken
Copy link

STRATZ-Ken commented Dec 22, 2017

Same issue as @jianxuanbing except I am trying for the NetworkAdapaters.

I am running it on Windows 2012 R2 in a Hyper-V VM in 64 Bit. If any of this helps.

@wspait
Copy link

wspait commented Dec 26, 2017

We are also having the same issue as @jianxuanbing. We are using version 4.5.0-preview1-25914-04. This code works fine locally (Windows 10), but not when run from a clean Server 2012 R2 or Server 2016 Windows image.

static void Main(string[] args)
 {
        try
        {
            Console.WriteLine("Calling System.Management code");
            ManagementObjectSearcher searcher = new ManagementObjectSearcher(@"root\CIMV2", "SELECT * FROM Win32_SystemEnclosure");
            Console.WriteLine("Application complete");
        }
        catch(Exception ex)
        {
            Console.WriteLine(ex);
        }     
}

This is the output:

Calling System.Management code
System.TypeInitializationException: The type initializer for 'System.Management.
WmiNetUtilsHelper' threw an exception. ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Management.WmiNetUtilsHelper..cctor()
   --- End of inner exception stack trace ---
   at System.Management.MTAHelper.IsNoContextMTA()
   at System.Management.MTAHelper.CreateInMTA(Type type)
   at System.Management.ManagementPath.CreateWbemPath(String path)
   at System.Management.ManagementObjectSearcher..ctor(String scope, String queryString)
   at WmiNetUtilsHelperTest.Program.Main(String[] args)

@danmoseley
Copy link
Member

@wspait the S.Management package requires a native binary that is included in the full.NET Framework 4.x. The error message could be clearer (please open a new issue). Meanwhile please add 4.x to your images.

@STRATZ-Ken
Copy link

@danmosemsft All of my machines have .Net 4.6.1 installed as that is what I use to run my program on. So it should have that dependency. I cant seem to make the error bigger either, myself.

@danmoseley
Copy link
Member

Looks like if .NET 4.x is not found it would have bailed out cleanly. It gets further. It could be here:

                procAddr = Interop.Kernel32.GetProcAddress(loadLibrary, "Initialize");
                if( procAddr != IntPtr.Zero)
                {
                    Initialize_f  =(Initialize) Marshal.GetDelegateForFunctionPointer(procAddr, typeof(Initialize));
                }

        Initialize_f(CompatSwitches.AllowIManagementObjectQI);

The Initialize_f call is accidentally outside of the braces.

Although I would expect problems after this point anyway -- if the LL/GPA is failing.

@danmoseley
Copy link
Member

danmoseley commented Dec 27, 2017

I set up a Server 2012 R2 machine (which has 4.7:2117.0 which I think is 4.7.1) and your code works fine.

@STRATZ-Ken @wspait are either of you willing to debug? Symbols and sources should be available. The code to put a breakpoint on is the static constructor of WmiNetUtilsHelper.

@danmoseley
Copy link
Member

To keep orgnaized I moved this to an issue https://github.com/dotnet/corefx/issues/26081 ... please continue discussion there not here

@dotnet dotnet locked as resolved and limited conversation to collaborators Jan 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.