1

In a windows service I know I have a memory leak. How I know is outside the scope of this question but you can see the initial question here

I have a windows service with some classes:

public partial class VLSService : ServiceBase
{

   ReportedContentProcess reportedContent;

   protected override void OnStart(string[] args)
   {
        //when a user reports a video
        reportedContent = new ReportedContentProcess();
        reportedContent.ProcessTimer.Elapsed += new ElapsedEventHandler(ReportedContentTimer_Elapsed);

   }

   void ReportedContentTimer_Elapsed(object sender, ElapsedEventArgs e)
   {
        reportedContent = new ReportedContentProcess();
        reportedContent.Process();
        reportedContent.ProcessReinstated();
   }

}


public class ReportedContentProcess : ProcessBase
    {

        //location on the library
        string libraryArchivedFilePath;
        string libraryRealHiFilePath;
        string libraryRealLoFilePath;
        string libraryFlashHiFilePath;
        string libraryFlashLoFilePath;

        //location on the reported folder
        string reportedContentArchivedFilePath;
        string reportedContentRealHiFilePath;
        string reportedContentRealLoFilePath;
        string reportedContentFlashHiFilePath;
        string reportedContentFlashLoFilePath;

        string reportedContentFolderPath;
        static EmailSettings emailSettings;

        /// <summary>
        /// This process will move reported content out of the 'real' and 'flash' mounted folders on the 
        /// hard drive/ storeage location so they cannot under any circumstances be got to by any users
        /// of the library.
        /// </summary>
        public ReportedContentProcess(): base(1021)
        {
            DirectoryInfo arciveFolderPathInfo = new DirectoryInfo(fileSystemReferencesForService.ArchiveDir);
            DirectoryInfo contentFolder = arciveFolderPathInfo.Parent;
            reportedContentFolderPath = contentFolder.FullName.ToString() + @"\ReportedContent\";
            emailSettings = settingsManagerForService.GetEmailSettings();
        }

        public override void Process()
        {

            if (!EnumsAndConstants.ApplicationLocks.ReportedContentProcessRunning)
            {

                EnumsAndConstants.ApplicationLocks.ReportedContentProcessRunning = true;

                videosToProcess = contentManagerForService.GetReportedVideos(false);

                //get the reportedvideo object for this video
                CreateReportedVideoContentFolder();

                ReportedVideo reportedVideo;

                foreach (Video v in videosToProcess)
                {

                    string flashVideoExt = string.Empty;

                    if (v.IsAudio)
                    {
                        flashVideoExt = ".mp3";
                    }
                    else
                    {
                        flashVideoExt = ".mp4";
                    }

                    //library location of each file for video
                    libraryArchivedFilePath = fileSystemReferencesForService.ArchiveDir + v.LocalFile;

                    libraryRealHiFilePath = fileSystemReferencesForService.RealDir + v.Url.ToString() + "_hi.rm";
                    libraryRealLoFilePath = fileSystemReferencesForService.RealDir + v.Url.ToString() + "_lo.rm";
                    libraryFlashHiFilePath = fileSystemReferencesForService.FlashDir + v.Url.ToString() + "_hi" + flashVideoExt;
                    libraryFlashLoFilePath = fileSystemReferencesForService.FlashDir + v.Url.ToString() + "_lo" + flashVideoExt;

                    //new location for file to go to
                    reportedContentArchivedFilePath = reportedContentFolderPath + v.LocalFile;


}

}


/// <summary>
/// A base class that holds all the Global objects for any process that operates under the 
/// service. This process works with 
/// </summary>
public abstract class ProcessBase
{
    public Timer processTimer;
    public Timer ProcessTimer{get{ return processTimer;}set{processTimer=value;}}

    protected SqlConnection connection;
    protected VlsContent contentManagerForService;
    protected VlsSecurity securityManagerForService;
    protected VlsSettings settingsManagerForService;
    protected FileSystemReferences fileSystemReferencesForService;
    protected List<Video> videosToProcess;
    protected ExeReferences exeReferenecesForService;
    protected GeneralSettings generalSettingsForService;

    public abstract void Process();


//sets up all the common objects
public ProcessBase() 
{

    connection = new SqlConnection(ConfigurationManager.ConnectionStrings["Db"].ToString());
    contentManagerForService = new VlsContent(connection);
    settingsManagerForService = new VlsSettings(connection);
    securityManagerForService = new VlsSecurity(connection);
    fileSystemReferencesForService = settingsManagerForService.GetFileSystemReferences();
    exeReferenecesForService = settingsManagerForService.GetExeReferences();
    generalSettingsForService = settingsManagerForService.GetGeneralSettings();

}


//This constructor will call the default constructor ^
protected ProcessBase(long intervalArg) : this()
{
    processTimer = new Timer(intervalArg);
    processTimer.Enabled = true;

}

}

After profiling this code it seems that this is causing a memory leak. What im wondering is why?

Im thinking that the problematic line is:

reportedContent = new ReportedContentProcess(); [located in the event handler]

But I cant really see why. Surely it will creat pointer in memory called 'reportedContent' then when the above is called it will place actual value on the heap with new values for the members of ReportedContentProcess(). Then when the event handler is run again after about 1 second it will then just replace the GC root pointer 'reportedContent' with a new allocated heap item for the ReportedContentProcess() class. Then the old one (and all of its now abandoned child objects will be garbaged collected as their root is no longer referenced by the call stack..? This should just happen over and over again (out with the old in with the new) style.

Hope some can help I sort of hope this is the problem so I can fix it but want to check before I start re-factoring code.

The profile is here:

enter image description here

10
  • 2
    What brings you to the conclusion that you have a memory leak? Jun 28, 2011 at 10:16
  • Well not likely that they would be garbage collected immediately
    – V4Vendetta
    Jun 28, 2011 at 10:21
  • I know I have a memory leak because the task manager says the service has 5GB of memory after 2 days of running.
    – Exitos
    Jun 28, 2011 at 10:23
  • just curious OnStart will be called only once at start of service? Jun 28, 2011 at 10:25
  • Hi Haris yes thats correct just the once
    – Exitos
    Jun 28, 2011 at 10:31

2 Answers 2

1

I don't know what ProcessBase is but I am thinking it is where your problem is.

Does it deal with anything outside of the .NET/C# environment? Should it implement an IDisposable interface and should the existing ReportedProcess be disposed before a new one is created.

Also, you create an event handler to the initial ReportedProcess in OnStart(). You then lose a reference to this with the first event by assigning a new instance to its reference, however the initial ReportedProcess is kept alive by the fact that it has a contained object that has a subscribed event handler, it will not be garbage collected until the end of the programme and is technically a memory leak.

If somewhere in the ReportedProcess class you apply the same strategy with regards to cleaning up event handlers then there is your problem.

** UPDATED ANSWER **

Just looking at your updated code, it's missing ProcessReinstated() code, but I think it maybe the fact that you do not explicitly Close your SqlConnection. Your ProcessBase should implement an IDisposable interface and your timer code should explicitly dispose of existing ReportedProcess classes before instantiating new ones, this is the best strategy with things like Database connections and sockets and other external connections.

1
  • Hi Adrian I have updated the code with the ProcessBase class!
    – Exitos
    Jun 28, 2011 at 10:56
0

It doesn’t seem to have a memory leak. Do you have any code similar to this:

reportedContent.ProcessTimer.Elapsed += new ElapsedEventHandler(ReportedContentTimer_Elapsed);

If you have then you have a reference to all of the reportedContect that you have created and GC doesn't release them.

3
  • I have the once in the OnStart method thats why I dont understand why its not working?, surely its only happening once so why does it keep hold of the obejects and create more.
    – Exitos
    Jun 28, 2011 at 11:09
  • 1
    If it happening only once, why don't you separate it from Process and put it in a class of its own (create a class that has only timer section of ProcessBase). In this case it is much clearer and there is cohesion between your classes.
    – user818967
    Jun 28, 2011 at 11:21
  • Isn't it the other way around? The ReportedContentProcess has a reference to VLSService. Which means that the VLSService cannot be GCed as long as the ReportedContentProcess is around (but the latter can be GCed itself). But we don't care about that.
    – Jon
    Jun 28, 2011 at 11:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.