Утечка памяти при использовании потоков - PullRequest
2 голосов
/ 27 января 2009

У меня, кажется, утечка памяти в этом куске кода. Это консольное приложение, которое создает несколько классов (WorkerThread), каждый из которых записывает данные в консоль с заданными интервалами. Для этого используется Threading.Timer, поэтому запись в консоль выполняется в отдельном потоке (TimerCallback вызывается в отдельном потоке, взятом из ThreadPool). Чтобы усложнить ситуацию, класс MainThread подключается к событию Changed FileSystemWatcher; при изменении файла test.xml классы WorkerThread воссоздаются.

Каждый раз, когда файл сохраняется (каждый раз, когда WorkerThread и, следовательно, Timer воссоздается), память в диспетчере задач увеличивается (использование памяти, а иногда и размер виртуальной машины); более того, в .Net Memory Profiler (v3.1) число нераспределенных экземпляров класса WorkerThread увеличивается на два (хотя это может быть красная сельдь, потому что я читал, что в .Net Memory Profiler была ошибка, из-за которой он пытался обнаружить распоряжаться классами.

В любом случае, вот код - кто-нибудь знает, что не так?

EDIT : я переместил создание классов из обработчика событий FileSystemWatcher.Changed, что означает, что классы WorkerThread всегда создаются в одном потоке. Я добавил защиту статическим переменным. Я также предоставил информацию о потоках, чтобы более четко показать, что происходит, и обменивался таймерами с использованием явного потока; однако память все еще течет! Использование памяти постоянно увеличивается (это просто из-за дополнительного текста в окне консоли?), И размер виртуальной машины увеличивается, когда я меняю файл. Вот последняя версия кода:

РЕДАКТИРОВАТЬ Похоже, что это в первую очередь проблема с консолью, использующей память, когда вы пишете в нее. Все еще существует проблема с явно написанными потоками, увеличивающими использование памяти. См. мой ответ ниже .

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
}

Ответы [ 6 ]

8 голосов
/ 27 января 2009

У вас есть два вопроса, оба отдельных:

В обработчике 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. A thread1.Stop ()
  2. A thread2.Stop ()
  3. B thread1.Stop ()
  4. B thread2.Stop ()
  5. A thread1 = новый WorkerThread ()
  6. A thread2 = новый WorkerThread ()
  7. B thread1 = new WorkerThread ()
  8. B thread2 = новый WorkerThread ()

Теперь у вас есть 4 экземпляра WorkerThread в куче, но только две переменные, ссылающиеся на них, две, созданные A, просочились. Обработка событий и регистрация обратного вызова с помощью таймера означает, что утечки WorkerThreads сохраняются (в смысле GC), несмотря на то, что вы не имеете ссылки на них в своем коде. они остаются протекающими навсегда.

Есть и другие недостатки в дизайне, но это очень важный вопрос.

3 голосов
/ 29 января 2009

Нет, нет, нет, нет, нет, нет, нет. Никогда не используйте Thread.Abort ().

Прочитайте MSDN документы на нем.


Нить не может быть прервана немедленно или вообще не гарантирована. Такая ситуация может возникнуть, если поток выполняет неограниченное количество вычислений в блоках finally, которые вызываются как часть процедуры прерывания, тем самым бесконечно задерживая прерывание. Чтобы дождаться завершения потока, вы можете вызвать метод Join в потоке после вызова метода Abort, но нет гарантии, что ожидание закончится.


Правильный способ завершить поток - сообщить ему, что он должен завершиться, а затем вызвать 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;
        }
    }    
}
2 голосов
/ 16 февраля 2009

Ну, когда у нас было время снова разобраться, похоже, что утечка памяти - это нечто вроде красной селедки. Когда я прекращаю запись в консоль, использование памяти перестает увеличиваться .

Однако существует остающаяся проблема в том, что каждый раз, когда я редактирую файл test.xml (который запускает событие Changed в FileSystemWatcher, чей обработчик устанавливает флаги, которые вызывают обновление рабочих классов и, следовательно, потоков / таймеров остановился), память увеличивается примерно на 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
}
0 голосов
/ 27 января 2009

Вы никогда не прерываете потоки - используйте что-то вроде Process Explorer, чтобы проверить, увеличивается ли количество потоков, а также объем памяти. Добавьте вызов метода Abort () в метод Stop ().

Редактировать: Вы сделали, спасибо.

0 голосов
/ 27 января 2009

Фактические рабочие потоки не удаляются, когда происходит событие отслеживаемого файла. Я думаю, что я бы переписал это так, чтобы новые потоки не создавались, а переинициализировались. Вместо вызова Stop и повторного создания потоков, вызовите новый метод Restart, который просто останавливает и сбрасывает таймер.

0 голосов
/ 27 января 2009

Ну, вы никогда не вызываете dispose в экземплярах WorkerThread.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...