Memory leak when using NetFwTypeLib in .Net Core 3.1

665 Views Asked by At

I tracked down a memory leak in this IDS/IPS traffic monitor console app I've been working on. I thought it was entity framework, turns out it's the firewall code. The following code worked perfectly fine in .net 4.6.2 and it works mostly fine in .net core. However, it has a memory leak (specifically the second firewallRule line):

INetFwPolicy2 firewallPolicy = (INetFwPolicy2)Activator.CreateInstance(Type.GetTypeFromProgID("HNetCfg.FwPolicy2"));
INetFwRule firewallRule = firewallPolicy.Rules.OfType<INetFwRule>().Where(x => x.Name == fwRuleName).FirstOrDefault();

I have to restart the app daily as it will burn through as much memory as I let it. I've seen it as high as 12GB when it should be running around 35MB. It only takes a day or so to get that high too. So basically each call to firewallRule is eating ~5MB-10MB. Seems crazy a single or null record would use up that much memory.

I believe it's that firewallRule line because I've literally commented out every other line in the method that's being called (and the app for that matter), and when I finally comment out that line, the leak stops. Please let me know if my logic is flawed.

If you want to test it, you have to make a reference to NetFwTypeLib to make that code work.

Does anyone know why this is happening and how to remedy it?

Update:

Here is the original code for those that believe I haven't commented it all out:

public static void FirewallSetup(string ip, string countryCode, bool isIpsString)
        {

        //    FirewallControl fwCtrl = new FirewallControl();
        //    fwCtrl.Block(ip, countryCode, isIpsString);
        //}

        //private void Block(string ip, string countryCode, bool isIpsString)
        //{
            var remoteAddresses = "*";
            string fwRuleName;
            bool createNewFwRule = false;

            //var fwRuleSuffix = SQLControl.GetFirewallRuleSuffix(countryCode).ToString();
            //if (string.IsNullOrEmpty(fwRuleSuffix) || fwRuleSuffix == "0")
            //{
                fwRuleName = fwRuleNamePrefix + countryCode;
            //}
            //else
            //{
            //    fwRuleName = fwRuleNamePrefix + countryCode + fwRuleSuffix;
            //}

            INetFwPolicy2 firewallPolicy = (INetFwPolicy2)Activator.CreateInstance(Type.GetTypeFromProgID("HNetCfg.FwPolicy2"));
            INetFwRule firewallRule = firewallPolicy.Rules.OfType<INetFwRule>().Where(x => x.Name == fwRuleName).FirstOrDefault();

            //if (firewallRule == null)
            //{
            //    createNewFwRule = true;
            //}
            //else
            //{
            //    //We need the following for creating the remoteAddresses string below
            //    //but, also need to count as Windows has a 5000 ip limit per rule
            //    remoteAddresses = firewallRule.RemoteAddresses;
            //    string[] aRemoteAddresses = remoteAddresses.Split(",");
            //    int remoteAddressesCount = aRemoteAddresses.Length;
            //    //Log.Debug(">>>Firewall remote addresses scope=" + remoteAddresses);
            //    Log.Warning(">>>" + fwRuleName + " Firewall remote addresses count:" + remoteAddressesCount);

            //    //5000 would be 0 to 4999 I think?
            //    if (remoteAddressesCount >= 4999)
            //    {
            //        //If remote ip scope is 5000, create a new fw rule
            //        var newFwRuleNameSuffix = SQLControl.GetNewFirewallRuleSuffix(countryCode).ToString();
            //        fwRuleName = fwRuleNamePrefix + countryCode + newFwRuleNameSuffix;

            //        createNewFwRule = true;
            //    }
            //}

            //If necessary, we create a new rule
            //TODO: Create another method for this?
            //if (createNewFwRule)
            //{
            //    firewallRule = (INetFwRule)Activator.CreateInstance(Type.GetTypeFromProgID("HNetCfg.FWRule"));
            //    firewallRule.Name = fwRuleName;
            //    firewallPolicy.Rules.Add(firewallRule);
            //    firewallRule.Description = "Block inbound traffic from " + countryCode;
            //    firewallRule.Profiles = (int)NET_FW_PROFILE_TYPE2_.NET_FW_PROFILE2_ALL;
            //    firewallRule.Protocol = (int)NET_FW_IP_PROTOCOL_.NET_FW_IP_PROTOCOL_TCP;
            //    firewallRule.Direction = NET_FW_RULE_DIRECTION_.NET_FW_RULE_DIR_IN;
            //    firewallRule.Action = NET_FW_ACTION_.NET_FW_ACTION_BLOCK;
            //    firewallRule.Enabled = true;
            //    //firewallRule.RemoteAddresses = ip;
            //    //firewallPolicy.Rules.Add(firewallRule); //throws error and not needed anyway
            //    //firewallRule.LocalPorts = "4000";
            //    //firewallRule.Grouping = "@firewallapi.dll,-23255";
            //    //firewallRule.Profiles = firewallPolicy.CurrentProfileTypes;
            //}

            //if (isIpsString || remoteAddresses == "*")
            //{
            //    firewallRule.RemoteAddresses = ip;
            //}
            //else
            //{
            //    firewallRule.RemoteAddresses = remoteAddresses + "," + ip;
            //}
        }

In production, it simply executes this method "as is" and just eats memory. Again, it is specific to .Net Core as I ran this same code in .Net 4.6.1 (un-commented) for more than a year without issue.

Now that I think about it and have commented it out, I can also put it through some loop and troubleshoot it here internally. It's a race...

The server it's currently running on is Windows Server 2016.

Update 2: It's reproducible in Windows 10 also. Here is a the entire code to reproduce the error:

using System;
using System.Linq;
using NetFwTypeLib;

namespace FirewallLeakTesterCore
{
    class Program
    {
        static void Main(string[] args)
        {
            int count = 0;
            while (count < 1000)
            {
                Console.WriteLine(count += 1);
                INetFwPolicy2 firewallPolicy = (INetFwPolicy2)Activator.CreateInstance(Type.GetTypeFromProgID("HNetCfg.FwPolicy2"));
                INetFwRule firewallRule = firewallPolicy.Rules.OfType<INetFwRule>().Where(x => x.Name == "firewallRuleName").FirstOrDefault();

            }
            Console.ReadKey();
        }
    }
}

And here is the result:

Visual Studio Diagnostic Tools:

Visual Studio Diagnostic Tools

Seems like a Microsoft bug? I guess I'll try and tackle one of these memory profilers, but even if I discover the issue, what can I do? This is a basic call to a basic windows library which I think is why some people in the comments are missing the point.

I'm not asking for anyone to memory profile my app or track down the issue (and never was... I've already done that). I'm asking for alternate solutions. Is there possibly a structure or linq issue or other statement entirely I can use?

If you want to leave the question closed, then so be it, but I have a hard time believing no one else is going to run into this.

1

There are 1 best solutions below

2
Walter Verhoeven On

I see a lot of comments in regards to your memory profiling and not a lot about addressing your problem... Here is an article you could use to test for memory leaks from JetBrains https://blog.jetbrains.com/dotnet/2018/10/04/unit-testing-memory-leaks-using-dotmemory-unit/

As to your problem, I guess you might have a look at Marshal.ReleaseComObject

I would release any COM call with something like this:

if (_netFwPolicy is not null && OperatingSystem.IsWindows())
{
    Marshal.ReleaseComObject(_netFwPolicy);
}

But what you really need to consider is wrapping the instances in a class that handles IDispose, this way any foreach will free the item as well, looping over the items in your policy will become an issue when used like this...

I know there are a lot of samples doing what you are doing on the internet, which doesn't by definition make it a good idea, perhaps consider playing with this wrapper.

public interface IFireWallRule :INetFwRule, IDisposable
{

}

class FireWallRule: IFireWallRule
{
    private INetFwRule? _data;
    private bool _disposedValue;

    public FireWallRule(INetFwRule data)
    {
        _data=data;
    }



    internal INetFwRule Root => _data?? throw new ObjectDisposedException("This firewall rule is already disposed and can't be accessed");
    public string Name { get => Root.Name; set => Root.Name=value; }
    public string Description { get => Root.Description; set => Root.Description=value; }
    public string ApplicationName { get => Root.ApplicationName; set => Root.ApplicationName=value; }
    public string serviceName { get => Root.serviceName; set => Root.serviceName=value; }
    public int Protocol { get => Root.Protocol; set => Root.Protocol=value; }
    public string LocalPorts { get => Root.LocalPorts; set => Root.LocalPorts=value; }
    public string RemotePorts { get => Root.RemotePorts; set => Root.RemotePorts=value; }
    public string LocalAddresses { get => Root.LocalAddresses; set => Root.LocalAddresses=value; }
    public string RemoteAddresses { get => Root.RemoteAddresses; set => Root.RemoteAddresses=value; }
    public string IcmpTypesAndCodes { get => Root.IcmpTypesAndCodes; set => Root.IcmpTypesAndCodes=value; }
    public NET_FW_RULE_DIRECTION_ Direction { get => Root.Direction; set => Root.Direction=value; }
    public dynamic Interfaces { get => Root.Interfaces; set => Root.Interfaces=value; }
    public string InterfaceTypes { get => Root.InterfaceTypes; set => Root.InterfaceTypes=value; }
    public bool Enabled { get => Root.Enabled; set => _data.Enabled=value; }
    public string Grouping { get => Root.Grouping; set => _data.Grouping=value; }
    public int Profiles { get => Root.Profiles; set => _data.Profiles=value; }
    public bool EdgeTraversal { get => Root.EdgeTraversal; set => Root.EdgeTraversal=value; }
    public NET_FW_ACTION_ Action { get => Root.Action; set => Root.Action=value; }




    protected virtual void Dispose(bool _)
    {
        if (!_disposedValue)
        {
            if (_data is not null && OperatingSystem.IsWindows())
            {
                Marshal.ReleaseComObject(_data);
                _data=null;
            }


            _disposedValue=true;
        }
    }


    ~FireWallRule()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

To make life "easy" I'd consider some extension methods to make working with the Wrapper class easier

internal static class FireWallExcensionMethods
{
    public static void Add(this INetFwPolicy2 policy, FireWallRule rule)
    {
        policy.Rules.Add(rule.Root);
    }
    public static void Add(this INetFwRules rules, FireWallRule rule)
    {
        rules.Add(rule.Root);
    }
    public static IEnumerable<FireWallRule> GetRules(this INetFwPolicy2 policy)
    {
        foreach (INetFwRule rule in policy.Rules)
        {
            yield return new FireWallRule(rule);
        }

    }
}