Memory leak, file in use by another process exception

Question

This application seems to eat a lot of memory. Maybe I'm not managing the memory properly. Can some please figure out what code needs to be optimized. And secondly, file copying is not working properly. It sometimes throws an exception, cannot access file being used by another process, this exception is being thrown at both ends the watch folder and the destination folder where I'm copying. Let me give you a brief of what I'm trying to achieve here.

I have a system that would give me an xml file in ansi format. This file would be updated regularly maybe every 3-4mins sometimes even 10-20secs. Now I'm watching this folder, as soon as it is changed I convert it to UTF-8 and copy this to another server via sftp. This sftp folder is mapped on this same machine where the conversion is happening. So the problems I'm facing is the exception it throws, can't access the file being used by another process, after a while this get's cleared. And even the memory exception, that the system is run out of memory. It's leaking memory as well. Starts at 5k, after a few hours reaches 1.2gb of memory usage. Now I need to run 3 of these similar programs watching 3 different folders. Any clues to my problems ?

class Test
{
    class Class1
    {
        private static FileSystemWatcher watcher = new FileSystemWatcher();

        public static void Main()
        {
            WatchFile();
            Console.ReadLine();
        }

        private static void WatchFile()
        {
            watcher.Path = @"c:\test";
            watcher.NotifyFilter = NotifyFilters.LastWrite;
            watcher.Filter = "*.xml";
            watcher.Changed += new FileSystemEventHandler(convert);
            watcher.Error += new ErrorEventHandler(WatcherError);
            watcher.EnableRaisingEvents = true;

            Console.WriteLine("Press \'q\' to quit.");
            Console.WriteLine("Press \'q\' to quit.");
            while (Console.Read() != 'q') ;
        }

        public static string CrL = "\r\n";

        private static void convert(object source, FileSystemEventArgs f)
        {
           string FileName = f.FullPath;
           string destinationFile = @"z:\xml\test.xml";

           Thread.Sleep(2000);
           try
           {
                watcher.EnableRaisingEvents = false;
                Encoding utf8 = new UTF8Encoding(false);
                Encoding ansi = Encoding.GetEncoding(1256);
                Thread.Sleep(2000);

                string xml = File.ReadAllText(FileName, ansi);
                XDocument xmlDoc = XDocument.Parse(xml);
                File.WriteAllText(FileName, @"<?xml version=""1.0"" encoding=""utf-8""?>" + xmlDoc.ToString(), utf8);

                 if (File.Exists(destinationFile))
                     File.Delete(destinationFile);
                 File.Copy(FileName, destinationFile,true);
                 Console.WriteLine("File Copied"); // for troubleshoooting only
                 Console.Write(CrL);
            }
            catch (Exception e)
            {
                Console.WriteLine("The process failed: {0}", e.ToString());
            }
            finally
            {
                watcher.EnableRaisingEvents = true;
            }
        }

        private static void WatcherError(object source, ErrorEventArgs e)
        {
            Exception watchException = e.GetException();
            watcher = new FileSystemWatcher();
            while (!watcher.EnableRaisingEvents)
            {
                try
                {
                    WatchFile();
                    Console.WriteLine("I'm Back!!");
                }
                catch
                {
                    Thread.Sleep(2000);
                }
            }
        }
    }  
}

Answer 1

Every time you get an error, you call WatchFile() again, which adds the convert and WatcherError methods to the Changed and Error invocation lists again. That would explain a slow memory leak. The delegate invocation lists keep growing.

Because the events are raised on pool threads, it's possible for your code to be processing multiple changed events concurrently.

Your error handler should just re-enable the events (i.e. Watcher.EnableRaisingEvents = true;) It certainly shouldn't be adding event handlers again.

You also need to synchronize access in your convert method. You could do it with a lock, but probably a better idea would be with a Monitor.TryEnter, like this:

private static object lockObject = new Object();
private static void convert(object source, FileSystemEventArgs f)
{
    if (!Monitor.TryEnter(lockObject))
    {
        // unable to get lock, return.
        return;
    }
    try
    {
        // do stuff here
    }
    finally
    {
        Monitor.Exit(lockObject);
    }