2

我似乎在这段代码中有内存泄漏。它是一个控制台应用程序,它创建了几个类(WorkerThread),每个类都以指定的时间间隔写入控制台。Threading.Timer 用于执行此操作,因此写入控制台是在单独的线程中执行的(在从 ThreadPool 获取的单独线程中调用 TimerCallback)。更复杂的是,MainThread 类与 FileSystemWatcher 的 Changed 事件挂钩;当 test.xml 文件更改时,会重新创建 WorkerThread 类。

每次保存文件时(每次重新创建 WorkerThread 并因此重新创建 Timer),任务管理器中的内存都会增加(Mem Usage,有时还会增加 VM Size);此外,在 .Net Memory Profiler (v3.1) 中,WorkerThread 类的 Undisposed Instances 增加了两个(但这可能是一个红鲱鱼,因为我读过 .Net Memory Profiler 有一个错误,它很难检测到处置的类。

无论如何,这是代码 - 有人知道出了什么问题吗?

编辑:我已经将类创建移出 FileSystemWatcher.Changed 事件处理程序,这意味着 WorkerThread 类总是在同一个线程中创建。我为静态变量添加了一些保护。我还提供了线程信息以更清楚地显示正在发生的事情,并且一直在使用 Timer 与使用显式线程进行交换;但是,内存仍在泄漏!Mem Usage 一直在缓慢增加(这仅仅是由于控制台窗口中的额外文本吗?),并且当我更改文件时 VM Size 会增加。这是最新版本的代码:

编辑这似乎主要是控制台使用内存的问题,因为您写入它。显式编写的线程仍然存在增加内存使用量的问题。请参阅下面的答案

class Program
{
    private static List<WorkerThread> threads = new List<WorkerThread>();

    static void Main(string[] args)
    {
        MainThread.Start();

    }
}

public class MainThread
{
    private static int _eventsRaised = 0;
    private static int _eventsRespondedTo = 0;
    private static bool _reload = false;
    private static readonly object _reloadLock = new object();
    //to do something once in handler, though
    //this code would go in onStart in a windows service.
    public static void Start()
    {
        WorkerThread thread1 = null;
        WorkerThread thread2 = null;

        Console.WriteLine("Start: thread " + Thread.CurrentThread.ManagedThreadId);
        //watch config
        FileSystemWatcher watcher = new FileSystemWatcher();
        watcher.Path = "../../";
        watcher.Filter = "test.xml";
        watcher.EnableRaisingEvents = true;
        //subscribe to changed event. note that this event can be raised a number of times for each save of the file.
        watcher.Changed += (sender, args) => FileChanged(sender, args);

        thread1 = new WorkerThread("foo", 10);
        thread2 = new WorkerThread("bar", 15);

        while (true)
        {
            if (_reload)
            {
                //create our two threads.
                Console.WriteLine("Start - reload: thread " + Thread.CurrentThread.ManagedThreadId);
                //wait, to enable other file changed events to pass
                Console.WriteLine("Start - waiting: thread " + Thread.CurrentThread.ManagedThreadId);
                thread1.Dispose();
                thread2.Dispose();
                Thread.Sleep(3000); //each thread lasts 0.5 seconds, so 3 seconds should be plenty to wait for the 
                                    //LoadData function to complete.
                Monitor.Enter(_reloadLock);
                thread1 = new WorkerThread("foo", 10);
                thread2 = new WorkerThread("bar", 15);
                _reload = false;
                Monitor.Exit(_reloadLock);
            }
        }
    }

    //this event handler is called in a separate thread to Start()
    static void FileChanged(object source, FileSystemEventArgs e)
    {
        Monitor.Enter(_reloadLock);
        _eventsRaised += 1;
        //if it was more than a second since the last event (ie, it's a new save), then wait for 3 seconds (to avoid 
        //multiple events for the same file save) before processing
        if (!_reload)
        {
            Console.WriteLine("FileChanged: thread " + Thread.CurrentThread.ManagedThreadId);
            _eventsRespondedTo += 1;
            Console.WriteLine("FileChanged. Handled event {0} of {1}.", _eventsRespondedTo, _eventsRaised);
            //tell main thread to restart threads
            _reload = true;
        }
        Monitor.Exit(_reloadLock);
    }
}

public class WorkerThread : IDisposable
{
    private System.Threading.Timer timer;   //the timer exists in its own separate thread pool thread.
    private string _name = string.Empty;
    private int _interval = 0;  //thread wait interval in ms.
    private Thread _thread = null;
    private ThreadStart _job = null;

    public WorkerThread(string name, int interval)
    {
        Console.WriteLine("WorkerThread: thread " + Thread.CurrentThread.ManagedThreadId);
        _name = name;
        _interval = interval * 1000;
        _job = new ThreadStart(LoadData);
        _thread = new Thread(_job);
        _thread.Start();
        //timer = new Timer(Tick, null, 1000, interval * 1000);
    }

    //this delegate instance does NOT run in the same thread as the thread that created the timer. It runs in its own
    //thread, taken from the ThreadPool. Hence, no need to create a new thread for the LoadData method.
    private void Tick(object state)
    {
        //LoadData();
    }

    //Loads the data. Called from separate thread. Lasts 0.5 seconds.
    //
    //private void LoadData(object state)
    private void LoadData()
    {
        while (true)
        {
            for (int i = 0; i < 10; i++)
            {
                Console.WriteLine(string.Format("Worker thread {0} ({2}): {1}", _name, i, Thread.CurrentThread.ManagedThreadId));
                Thread.Sleep(50);
            }
            Thread.Sleep(_interval);
        }
    }

    public void Stop()
    {
        Console.WriteLine("Stop: thread " + Thread.CurrentThread.ManagedThreadId);
        //timer.Dispose();
        _thread.Abort();
    }


    #region IDisposable Members

    public void Dispose()
    {
        Console.WriteLine("Dispose: thread " + Thread.CurrentThread.ManagedThreadId);
        //timer.Dispose();
        _thread.Abort();
    }

    #endregion
}
4

6 回答 6

8

你有两个问题,都是分开的:

在 Watcher.Changed 的​​处理程序中,您调用 Thread.Sleep(3000); 这是您不拥有的线程的回调中的不良行为(因为它是由观察者拥有/使用的池提供的。但这不是问题的根源。这直接违反了使用指南

你到处都使用静力学,这很可怕,很可能导致你陷入这个问题:

static void test()
{
    _eventsRaised += 1;
    //if it was more than a second since the last event (ie, it's a new save), then wait for 3 seconds (to avoid 
    //multiple events for the same file save) before processing
    if (DateTime.Now.Ticks - _lastEventTicks > 1000)
    {
        Thread.Sleep(3000);
        _lastEventTicks = DateTime.Now.Ticks;
        _eventsRespondedTo += 1;
        Console.WriteLine("File changed. Handled event {0} of {1}.", _eventsRespondedTo, _eventsRaised);
        //stop threads and then restart them
        thread1.Stop();
        thread2.Stop();
        thread1 = new WorkerThread("foo", 20);
        thread2 = new WorkerThread("bar", 30);
    }
}

此回调可以在多个不同的线程上重复触发(它为此使用系统线程池)您的代码假定一次只有一个线程将执行此方法,因为可以创建线程但不能停止。

想象一下:线程 A 和 B

  1. 线程1.Stop()
  2. 一个thread2.Stop()
  3. B线程1.Stop()
  4. B thread2.Stop()
  5. 线程 1 = 新的 WorkerThread()
  6. 线程 2 = 新的 WorkerThread()
  7. B 线程 1 = 新的 WorkerThread()
  8. B 线程 2 = 新的 WorkerThread()

您现在在堆上有 4 个 WorkerThread 实例,但只有两个变量引用它们,由 A 创建的两个变量已经泄漏。定时器的事件处理和回调注册意味着这些泄漏的 WorkerThreads 保持活动状态(在 GC 意义上),尽管您在代码中没有引用它们。他们永远被泄露。

设计中还有其他缺陷,但这是一个关键缺陷。

于 2009-01-27T14:34:19.153 回答
3

不,不,不,不,不,不,不,不。永远不要使用 Thread.Abort()。

阅读MSDN 文档


不保证线程立即中止,或者根本不中止。如果线程在作为 abort 过程的一部分调用的 finally 块中执行无限量的计算,则可能会发生这种情况,从而无限期地延迟 abort。要等到线程中止,您可以在调用 Abort 方法后在线程上调用 Join 方法,但不能保证等待会结束。


结束线程的正确方法是向它发出它应该结束的信号,然后在该线程上调用 Join()。我通常会做这样的事情(伪代码):

public class ThreadUsingClass
{
    private object mSyncObject = new object();
    private bool mKilledThread = false;
    private Thread mThread = null;

    void Start()
    {
        // start mThread
    }

    void Stop()
    {
        lock(mSyncObject)
        {
            mKilledThread = true;
        }

        mThread.Join();
    }

    void ThreadProc()
    {
        while(true)
        {
            bool isKilled = false;
            lock(mSyncObject)
            {
                isKilled = mKilledThread;
            }
            if (isKilled)
                return;
        }
    }    
}
于 2009-01-29T16:02:47.613 回答
2

好吧,有一些时间再次调查这个问题,看来内存泄漏有点像红鲱鱼。当我停止写入控制台时,内存使用量停止增加

但是,还有一个问题是每次我编辑 test.xml 文件(它会在 FileSystemWatcher 上触发 Changed 事件,其处理程序设置的标志会导致工作类被更新,因此线程/计时器被停止),内存增加了大约 4K,前提是我使用的是显式线程,而不是计时器。当我使用定时器时,没有问题。但是,鉴于我宁愿使用计时器而不是线程,这对我来说不再是问题,但我仍然会对它发生的原因感兴趣。

请参阅下面的新代码。我创建了两个类 - WorkerThread 和 WorkerTimer,其中一个使用线程,另一个使用定时器(我尝试了两个定时器,System.Threading.Timer 和 System.Timers.Timer。在打开控制台输出的情况下,你可以看到这与引发滴答事件的线程有关)。只需注释/取消注释 MainThread.Start 的相应行即可使用所需的类。由于上述原因,建议将 Console.WriteLine 行注释掉,除非您想检查一切是否按预期工作。

class Program
{
    static void Main(string[] args)
    {
        MainThread.Start();

    }
}

public class MainThread
{
    private static int _eventsRaised = 0;
    private static int _eventsRespondedTo = 0;
    private static bool _reload = false;
    private static readonly object _reloadLock = new object();
    //to do something once in handler, though
    //this code would go in onStart in a windows service.
    public static void Start()
    {
        WorkerThread thread1 = null;
        WorkerThread thread2 = null;
        //WorkerTimer thread1 = null;
        //WorkerTimer thread2 = null;

        //Console.WriteLine("Start: thread " + Thread.CurrentThread.ManagedThreadId);
        //watch config
        FileSystemWatcher watcher = new FileSystemWatcher();
        watcher.Path = "../../";
        watcher.Filter = "test.xml";
        watcher.EnableRaisingEvents = true;
        //subscribe to changed event. note that this event can be raised a number of times for each save of the file.
        watcher.Changed += (sender, args) => FileChanged(sender, args);

        thread1 = new WorkerThread("foo", 10);
        thread2 = new WorkerThread("bar", 15);
        //thread1 = new WorkerTimer("foo", 10);
        //thread2 = new WorkerTimer("bar", 15);

        while (true)
        {
            if (_reload)
            {
                //create our two threads.
                //Console.WriteLine("Start - reload: thread " + Thread.CurrentThread.ManagedThreadId);
                //wait, to enable other file changed events to pass
                //Console.WriteLine("Start - waiting: thread " + Thread.CurrentThread.ManagedThreadId);
                thread1.Dispose();
                thread2.Dispose();
                Thread.Sleep(3000); //each thread lasts 0.5 seconds, so 3 seconds should be plenty to wait for the 
                //LoadData function to complete.
                Monitor.Enter(_reloadLock);
                //GC.Collect();
                thread1 = new WorkerThread("foo", 5);
                thread2 = new WorkerThread("bar", 7);
                //thread1 = new WorkerTimer("foo", 5);
                //thread2 = new WorkerTimer("bar", 7);
                _reload = false;
                Monitor.Exit(_reloadLock);
            }
        }
    }

    //this event handler is called in a separate thread to Start()
    static void FileChanged(object source, FileSystemEventArgs e)
    {
        Monitor.Enter(_reloadLock);
        _eventsRaised += 1;
        //if it was more than a second since the last event (ie, it's a new save), then wait for 3 seconds (to avoid 
        //multiple events for the same file save) before processing
        if (!_reload)
        {
            //Console.WriteLine("FileChanged: thread " + Thread.CurrentThread.ManagedThreadId);
            _eventsRespondedTo += 1;
            //Console.WriteLine("FileChanged. Handled event {0} of {1}.", _eventsRespondedTo, _eventsRaised);
            //tell main thread to restart threads
            _reload = true;
        }
        Monitor.Exit(_reloadLock);
    }
}

public class WorkerTimer : IDisposable
{
    private System.Threading.Timer _timer;   //the timer exists in its own separate thread pool thread.
    //private System.Timers.Timer _timer;
    private string _name = string.Empty;

    /// <summary>
    /// Initializes a new instance of the <see cref="WorkerThread"/> class.
    /// </summary>
    /// <param name="name">The name.</param>
    /// <param name="interval">The interval, in seconds.</param>
    public WorkerTimer(string name, int interval)
    {
        _name = name;
        //Console.WriteLine("WorkerThread constructor: Called from thread " + Thread.CurrentThread.ManagedThreadId);
        //_timer = new System.Timers.Timer(interval * 1000);
        //_timer.Elapsed += (sender, args) => LoadData();
        //_timer.Start();
        _timer = new Timer(Tick, null, 1000, interval * 1000);
    }

    //this delegate instance does NOT run in the same thread as the thread that created the timer. It runs in its own
    //thread, taken from the ThreadPool. Hence, no need to create a new thread for the LoadData method.
    private void Tick(object state)
    {
        LoadData();
    }

    //Loads the data. Called from separate thread. Lasts 0.5 seconds.
    //
    private void LoadData()
    {
        for (int i = 0; i < 10; i++)
        {
            //Console.WriteLine(string.Format("Worker thread {0} ({2}): {1}", _name, i, Thread.CurrentThread.ManagedThreadId));
            Thread.Sleep(50);
        }
    }

    public void Stop()
    {
        //Console.WriteLine("Stop: called from thread " + Thread.CurrentThread.ManagedThreadId);
        //_timer.Stop();
        _timer.Change(Timeout.Infinite, Timeout.Infinite);
        //_timer = null;
        //_timer.Dispose();
    }


    #region IDisposable Members

    public void Dispose()
    {
        //Console.WriteLine("Dispose: called from thread " + Thread.CurrentThread.ManagedThreadId);
        //_timer.Stop();
        _timer.Change(Timeout.Infinite, Timeout.Infinite);
        //_timer = null;
        //_timer.Dispose();
    }

    #endregion
}

public class WorkerThread : IDisposable
{
    private string _name = string.Empty;
    private int _interval = 0;  //thread wait interval in ms.
    private Thread _thread = null;
    private ThreadStart _job = null;
    private object _syncObject = new object();
    private bool _killThread = false;

    public WorkerThread(string name, int interval)
    {
        _name = name;
        _interval = interval * 1000;
        _job = new ThreadStart(LoadData);
        _thread = new Thread(_job);
        //Console.WriteLine("WorkerThread constructor: thread " + _thread.ManagedThreadId + " created. Called from thread " + Thread.CurrentThread.ManagedThreadId);
        _thread.Start();
    }

    //Loads the data. Called from separate thread. Lasts 0.5 seconds.
    //
    //private void LoadData(object state)
    private void LoadData()
    {
        while (true)
        {
            //check to see if thread it to be stopped.
            bool isKilled = false;

            lock (_syncObject)
            {
                isKilled = _killThread;
            }

            if (isKilled)
                return;

            for (int i = 0; i < 10; i++)
            {
                //Console.WriteLine(string.Format("Worker thread {0} ({2}): {1}", _name, i, Thread.CurrentThread.ManagedThreadId));
                Thread.Sleep(50);
            }
            Thread.Sleep(_interval);
        }
    }

    public void Stop()
    {
        //Console.WriteLine("Stop: thread " + _thread.ManagedThreadId + " called from thread " + Thread.CurrentThread.ManagedThreadId);
        //_thread.Abort();
        lock (_syncObject)
        {
            _killThread = true;
        }
        _thread.Join();
    }


    #region IDisposable Members

    public void Dispose()
    {
        //Console.WriteLine("Dispose: thread " + _thread.ManagedThreadId + " called from thread " + Thread.CurrentThread.ManagedThreadId);
        //_thread.Abort();
        lock (_syncObject)
        {
            _killThread = true;
        }
        _thread.Join();
    }

    #endregion
}
于 2009-02-16T11:45:12.077 回答
0

好吧,您实际上从未调用dispose过 WorkerThread 实例。

于 2009-01-27T13:05:23.000 回答
0

当监视的文件事件发生时,实际的工作线程不会被释放。我想我会重写它,这样就不会创建新线程,而是重新初始化它们。与其调用Stop和重新创建线程,不如调用一个新Restart方法来停止并重置计时器。

于 2009-01-27T13:07:28.673 回答
0

您永远不会终止线程 - 使用 Process Explorer 之类的东西来检查线程计数是否在增加以及内存。在您的 Stop() 方法中添加对 Abort() 的调用。

编辑:你做到了,谢谢。

于 2009-01-27T13:31:36.143 回答