Running windows service to watch service running grow memory (leak)

Question

I have checked all posts here, but can't find a solution for me so far. I did setup a small service that should only watch if my other services I want to monitor runs, and if not, start it again and place a message in the application eventlog.

The service itself works great, well nothing special :), but when I start the service it use around 1.6MB of RAM, and every 10 seconds it grow like 60-70k which is way to much to live with it. I tried dispose and clear all resources. Tried work with the System.Timers instead of the actual solution, but nothing really works as I want it, memory still grows.

No difference in debug or release version and I am using it on .Net 2, don't know if it make a difference to you 3,3.5 or 4.

Any hint?!

using System;
using System.IO;
using System.Diagnostics;
using System.ServiceProcess;
using System.Threading;
using System.Timers;

namespace Watchguard
{
  class WindowsService : ServiceBase
  {

    Thread mWorker;
    AutoResetEvent mStop = new AutoResetEvent(false);

    /// <summary>
    /// Public Constructor for WindowsService.
    /// - Put all of your Initialization code here.
    /// </summary>
    public WindowsService()
    {
        this.ServiceName = "Informer Watchguard";
        this.EventLog.Source = "Informer Watchguard";
        this.EventLog.Log = "Application";

      // These Flags set whether or not to handle that specific
        //  type of event. Set to true if you need it, false otherwise.
        this.CanHandlePowerEvent = false;
        this.CanHandleSessionChangeEvent = false;
        this.CanPauseAndContinue = false;
        this.CanShutdown = false;
        this.CanStop = true;

        if (!EventLog.SourceExists("Informer Watchguard"))
          EventLog.CreateEventSource("Informer Watchguard", "Application");
    }

    /// <summary>
    /// The Main Thread: This is where your Service is Run.
    /// </summary>
    static void Main()
    {
        ServiceBase.Run(new WindowsService());
    }

    /// <summary>
    /// Dispose of objects that need it here.
    /// </summary>
    /// <param name="disposing">Whether or not disposing is going on.</param>
    protected override void Dispose(bool disposing)
    {
        base.Dispose(disposing);
    }

    /// <summary>
    /// OnStart: Put startup code here
    ///  - Start threads, get inital data, etc.
    /// </summary>
    /// <param name="args"></param>
    protected override void OnStart(string[] args)
    {

      base.OnStart(args);

      MyLogEvent("Init");

      mWorker = new Thread(WatchServices);
      mWorker.Start();

    }

    /// <summary>
    /// OnStop: Put your stop code here
    /// - Stop threads, set final data, etc.
    /// </summary>
    protected override void OnStop()
    {

      mStop.Set();
      mWorker.Join();

      base.OnStop();

    }

    /// <summary>
    /// OnSessionChange(): To handle a change event from a Terminal Server session.
    ///   Useful if you need to determine when a user logs in remotely or logs off,
    ///   or when someone logs into the console.
    /// </summary>
    /// <param name="changeDescription"></param>
    protected override void OnSessionChange(SessionChangeDescription changeDescription)
    {
      base.OnSessionChange(changeDescription);
    }

    private void WatchServices()
    {

      string scName = "";

      ServiceController[] scServices;
      scServices = ServiceController.GetServices();

      for (; ; )
      {
        // Run this code once every 10 seconds or stop right away if the service is stopped
        if (mStop.WaitOne(10000)) return;
        // Do work...
        foreach (ServiceController scTemp in scServices)
        {

          scName = scTemp.ServiceName.ToString().ToLower();

          if (scName == "InformerWatchguard") scName = ""; // don't do it for yourself

          if (scName.Length > 8) scName = scName.Substring(0, 8);

          if (scName == "informer")
          {

            ServiceController sc = new ServiceController(scTemp.ServiceName.ToString());

            if (sc.Status == ServiceControllerStatus.Stopped)
            {

              sc.Start();
              MyLogEvent("Found service " + scTemp.ServiceName.ToString() + " which has status: " + sc.Status + "\nRestarting Service...");

            }

            sc.Dispose();
            sc = null;

          }
        }
      }

    }

    private static void MyLogEvent(String Message)
    {
      // Create an eEventLog instance and assign its source.
      EventLog myLog = new EventLog();
      myLog.Source = "Informer Watchguard";

      // Write an informational entry to the event log.
      myLog.WriteEntry(Message);
    }
  }
}

Answer 1

At a minimum, you need to do this in your logging code since EventLog needs to be Dispose()d. Seems like this resource could be reused rather than new-ed on every call. You could also consider using in your main loop for the ServiceController objects, to make your code more exception-safe.

private static void MyLogEvent(String Message)
{
  // Create an eEventLog instance and assign its source.
  using (EventLog myLog = new EventLog())
 {
   myLog.Source = "Informer Watchguard";

   // Write an informational entry to the event log.
   myLog.WriteEntry(Message);
 }
}

Answer 2

Your code may throw an exceptions inside loop, but these exception are not catched. So, change the code as follows to catch exceptions:

if (scName == "informer")
{
    try {
        using(ServiceController sc = new ServiceController(scTemp.ServiceName.ToString())) {
            if (sc.Status == ServiceControllerStatus.Stopped)
            {
                sc.Start();
                MyLogEvent("Found service " + scTemp.ServiceName.ToString() + " which has status: " + sc.Status + "\nRestarting Service...");
            }
        }
    } catch {
        // Write debug log here
    }
}

You can remove outer try/catch after investigating, leaving using statement to make sure Dispose called even if exception thrown inside.

Answer 3

This should be moved into the loop, since you don't want to keep a reference to old service handles for the life of your service:

ServiceController[] scServices = ServiceController.GetServices();

You also want to dispose of your reference to EventLog and to the ServiceController instances. As Artem points out, watch out for exceptions that are preventing you from doing this.

Since memory is going up every 10 seconds, it has to be something in your loop.

If memory goes up whether or not you write to the EventLog, then that is not the main problem.

Does memory used ever come down? Ie does the garbage collector kick in after awhile? You could test the GC's effect by doing a GC.Collect() before going back to sleep (though I'd be careful of using it in production).

Answer 4

I am not sure I understand the problem exactly. Is the service you are going to be monitoring always the same. It would appear from your code that the answer is yes, and if that is the case then you can simply create the ServiceController class instance passing the name of the service to the constructor. In your thread routine you want to continue looping until a stop is issued, and the WaitOne method call returns a Boolean, so a while loop seems to be appropriate. Within the while loop you can call the Refresh method on the ServiceController class instance to get the current state of the service. The event logging should simple require a call one of the static method EventLog.WriteEntry methods, at minimum passing your message and the source 'Informer Watchguard' The ServiceController instance can be disposed when you exit from the loop in the thread routine All this would mean you are creating fewer objects that need to be disposed, and therefore less likely that some resource leak will exist.