Бредисловие
Думаю каждый программист слышал или использовал shared pointers и встречался с идиомой ref counter для разделяемых ресурсов. По отдельности их использование не представляет сложности и опасности, но что может быть если совместить их вместе?
А как все начиналось...
Предположим мы расширяем существующий функционал программы. Архитектура программы содержит в себе некий "промежуточный" слой service, который состоит из классов-менеджеров. Т.е. для записи файла на CD есть класс CdWriterManager, для записи файлов на расшаренные папки ShareWriterManager и прочие. И вот нам дают задание написать класс для записи файлов на FTP.
Следуя уже сложившейся традиции, мы пишем следующий класс:
Как видно из кода, есть глобальная функция, возвращающая новый экземпляр класса. Т.е. в разный потоках могут быть разные объекты. И вот вы тестируете класс, и проверив, что все работает, гордые собой, отправляетесь "индексировать" Хабр :-)
Но тут выясняется, что наш класс должен как-то реагировать на внешние сигналы, или получать данные из других частей программы с помощью callbacks. Все callbacks классы в программе унаследованы от RefCounter.
После этого есть два пути: написать отдельный callback, унаследовав его от RefCounter, и сохранить объект этого типа в нашем менеджере. Или еще один "гениальный" способ: воспользоваться наследованием, дабы не плодить лишние классы и объекты
Поворот не туда
Да, да, сколько раз уже предупреждали о том, чтобы отдавать предпочтение композиции наследованию, но так ведь проще, и класс лишний писать не недо:
И после этого вы отдаете класс некоему менеджеру, обрабатываете события в функциях OnFinish, OnStart и все вроде работает. Но! Если объект FTPWriterManager будет разрушен, то случится беда.
А вот и Джонни...
Первым вызовется деструктор ~FTPWriterManager. В момент уничтожения FTPWriterManager будет вызван деструктор класса SomeEventHandler, и объект будет разрушен, (причем RefCounter::Counter не будет равен 0). Затем начнет уничтожаться объект FileManagerObj, который попытается уничтожить Handler, вызвав Release, но Handler уже указывает на не валидную память.
В моей программе эта ошибка спокойно жила себе, роняла программу лишь в дебаггере при закрытии диалога. Причем callstack был просто "фееричный".
Вывод
А вывод довольно прост: предпочитать композицию наследованию. Ну и почаще использовать assert :)
Думаю каждый программист слышал или использовал shared pointers и встречался с идиомой ref counter для разделяемых ресурсов. По отдельности их использование не представляет сложности и опасности, но что может быть если совместить их вместе?
А как все начиналось...
Предположим мы расширяем существующий функционал программы. Архитектура программы содержит в себе некий "промежуточный" слой service, который состоит из классов-менеджеров. Т.е. для записи файла на CD есть класс CdWriterManager, для записи файлов на расшаренные папки ShareWriterManager и прочие. И вот нам дают задание написать класс для записи файлов на FTP.
Следуя уже сложившейся традиции, мы пишем следующий класс:
- class FTPWriterManager
- {
- public:
- FTPWriterManager(){}
- virtual ~FTPWriterManager(){}
- void WriteFile(IFile::Ptr file)
- {
- //some logic here
- }
- typedef std::tr1::shared_ptr<FTPWriterManager> Ptr;
- };
- //get new instance of manager(not singleton!)
- FTPWriterManager::Ptr GetFtpWriterManager();
Как видно из кода, есть глобальная функция, возвращающая новый экземпляр класса. Т.е. в разный потоках могут быть разные объекты. И вот вы тестируете класс, и проверив, что все работает, гордые собой, отправляетесь "индексировать" Хабр :-)
Но тут выясняется, что наш класс должен как-то реагировать на внешние сигналы, или получать данные из других частей программы с помощью callbacks. Все callbacks классы в программе унаследованы от RefCounter.
- class RefCounter
- {
- private:
- volatile long Counter;
- public:
- RefCounter() : Counter(1)
- {
- }
- virtual ~RefCounter()
- {
- }
- virtual void AddRef()
- {
- atomic_increment(&Counter);
- }
- virtual void Release()
- {
- if (atomic_decrement(&Counter) == 0)
- {
- delete this;
- }
- }
- };
- class SomeHandler : public RefCounter
- {
- public:
- virtual void OnStart() = 0;
- virtual void OnFinish() = 0;
- };
После этого есть два пути: написать отдельный callback, унаследовав его от RefCounter, и сохранить объект этого типа в нашем менеджере. Или еще один "гениальный" способ: воспользоваться наследованием, дабы не плодить лишние классы и объекты
Поворот не туда
"Вот говорили мне - нельзя с утра до ночи смотреть телевизор - вот вам, пожалуйста"(с)
Да, да, сколько раз уже предупреждали о том, чтобы отдавать предпочтение композиции наследованию, но так ведь проще, и класс лишний писать не недо:
- class FileManager
- {
- public:
- ~virtual FileManager()
- {
- Handler->Release();
- }
- void SetCallback(SomeHandler* handler)
- {
- Handler = handler;
- }
- private:
- SomeHandler* Handler;
- };
- class FTPWriterManager : public SomeHandler
- {
- public:
- FTPWriterManager(){}
- virtual ~FTPWriterManager(){}
- void WriteFile(IFile::Ptr file)
- {
- //some logic here
- }
- void OnStart()
- {
- //...
- }
- void OnFinish()
- {
- //...
- }
- void Initialize()
- {
- FileManagerObj->SetCallback(this);
- }
- typedef std::tr1::shared_ptr<FTPWriterManager> Ptr;
- private:
- std::tr1::shared_ptr<FileManager> FileManagerObj;
- };
И после этого вы отдаете класс некоему менеджеру, обрабатываете события в функциях OnFinish, OnStart и все вроде работает. Но! Если объект FTPWriterManager будет разрушен, то случится беда.
А вот и Джонни...
Первым вызовется деструктор ~FTPWriterManager. В момент уничтожения FTPWriterManager будет вызван деструктор класса SomeEventHandler, и объект будет разрушен, (причем RefCounter::Counter не будет равен 0). Затем начнет уничтожаться объект FileManagerObj, который попытается уничтожить Handler, вызвав Release, но Handler уже указывает на не валидную память.
В моей программе эта ошибка спокойно жила себе, роняла программу лишь в дебаггере при закрытии диалога. Причем callstack был просто "фееричный".
Вывод
А вывод довольно прост: предпочитать композицию наследованию. Ну и почаще использовать assert :)
Комментариев нет:
Отправить комментарий