[Buttons & Timers] Деструктора на Wheel се извиква след като е освободен gTimerMgr

MartinBG 4803 Точки

[Buttons & Timers] Деструктора на Wheel се извиква след като е освободен gTimerMgr

Привет, колеги!

Докато правих заданието към "Buttons & Timers" (в общи линии съм следвал това, което сме правили на лекцията) забелязах, че при излизане от програмата AddressSanitizer-a хвърля следната грешка:

AddressSanitizer:DEADLYSIGNAL:

hashtable.h
SEGV on unknown address 0x000000000058 (pc 0x560f914507f4 bp 0x7ffd48a7e0a0 sp 0x7ffd48a7e080 T0)
...
std::unordered_set<int, std::hash, std::equal_to, std::allocator>::find(int const&) const
TimerManager::isActiveTimerId(int) const
TimerClient::isActiveTimerId(int) const
Wheel::~Wheel()
Game::~Game()
Engine::~Engine()
runApplication
main
__libc_start_main
_start

Проблемът е, че деструктора на Wheel:

Wheel::~Wheel() {
  if (isActiveTimerId(_rotateAnimationTimerId)) {
    stopTimer(_rotateAnimationTimerId);
  }
}

се извиква след рилийза на gTimerMgr и съответно при извикването на TimerClient::isActiveTimerId() той е nullptr.

Заобиколих го като добавих проверка за gTimerMgr:

bool TimerClient::isActiveTimerId(const int32_t timerId) const {
  return gTimerMgr && gTimerMgr->isActiveTimerId(timerId);
}

Някой друг натъквал ли се е на проблеми от подобно естество с проекта?

Тагове:

04/11/2021 23:35:14 C++ Applications Development

j.petrov_90 373 Точки

Best Answer

Привет, MartinBG,

Така е, бъга го има.
Забелязах го щом затворих стрийма. Оправих го при мен, но съм забравил да го кажа по време на следващото занятие.

Проверката, която си направил е вярна.
Точно поради тази причина си играхме в ManagerHandler-а да зануляваме пойтърите към Singleton глобалните класове след като унищожим дадения обект.
За да може в последствие човек да направи проста проверка дали този понтър е nullptr или не.

И сега възвиква въпроса - трябва ли на всякъде, където се използват тези Singletons да се прави тази проверка?
Краткия отговор е - не.
Можем да правим само тези проверки в destroy/destruct/unload функциите на класовете, които ги използват.
Ние сме гарантирали, че през останалото време обекта ще е жив.

Поздрави

05/11/2021 16:37:28 05/11/2021 17:03:59

MartinBG 4803 Точки

На последната лекция беше предложен като алтернативен вариант на nullptr проверките в методите на TimerClient, ManagerHandler да се създаде в runApplication()/main() и да се подаде (като ref/pointer) на Engine, а deinit-a му да се вика след този на Engine.

Това не променя поведението (на практика е същото като _handleManager-a да се създава директно от Engine както е сега) и няма да помогне, защото entity обектите (напр. Hero, Wheel) са независими и техните деструктори могат да бъдат извикани след като са унищожени/деинициализирани както ManagerHandler, така и Engine, както е видно и от този лог:

Engine::deinit()
Game::deinit()
Hero::deinit()
Wheel::deinit() 
ManagerHandler::deinit()
MusicManager::~MusicManager()
SoundManager::~SoundManager()
TimerManager::~TimerManager()
ResourceManager::~ResourceManager()
DrawManager::~DrawManager() 
Hero::~Hero()
Timer call TimerClient::isActiveTimerId: 2
AddressSanitizer:DEADLYSIGNAL

Възможно е entity обектите да бъдат унищожени преди ManagerHandler::deinit от някой компилатор/система, но няма как да разчитаме на това. Например, ако не викаме HandleManager::deinit() ръчно, а в деструктора на му:

ManagerHandler::~ManagerHandler() {
  deinit();
}

Може да имаме или да нямаме грешката според това как са подредени мембърите на Engine класа (и най-вероятно според компилатора/системата на която се изпълнява кода).

На моята система тази подредба не работи:

  Game _game;
  InputEvent _event;
  ManagerHandler _managerHandler;


///////////////////

Engine::deinit()
Game::deinit()
Hero::deinit()
Wheel::deinit() 
Engine::~Engine()
ManagerHandler::~ManagerHandler()
ManagerHandler::deinit()
MusicManager::~MusicManager()
SoundManager::~SoundManager()
TimerManager::~TimerManager()
ResourceManager::~ResourceManager()
DrawManager::~DrawManager() 
Game::~Game()
Hero::~Hero()
Timer call TimerClient::isActiveTimerId: 2
AddressSanitizer:DEADLYSIGNAL

Process finished with exit code 1

А с тази няма проблем:

  ManagerHandler _managerHandler;
  Game _game;
  InputEvent _event;


////////////////////////////////////

Engine::deinit()
Game::deinit()
Hero::deinit()
Wheel::deinit() 
Engine::~Engine()
Game::~Game()
Hero::~Hero()
Timer call TimerClient::isActiveTimerId: 2
Wheel::~Wheel() 
Timer call TimerClient::isActiveTimerId: 0
Timer call TimerClient::isActiveTimerId: 1
ManagerHandler::~ManagerHandler()
ManagerHandler::deinit()
MusicManager::~MusicManager()
SoundManager::~SoundManager()
TimerManager::~TimerManager()
ResourceManager::~ResourceManager()
DrawManager::~DrawManager() 

Process finished with exit code 1

Фактът, че зависим от подредбата на мембърите и от компилатора/системата прави кода твърде чуплив, а "решението" - ненадеждно.

Някой може да ли да предложи друго решение на проблема, освен nullptr проверки навсякъде, където ползваме глобалните статични обекти?

Как би изглеждало решение, при което не използваме глобални статични обекти и дали е практично въобще да се мисли в тази посока (какви нови проблеми ще ни докара този подход)?

10/11/2021 16:11:21 10/11/2021 16:53:44

j.petrov_90 373 Точки

Привет, Мартин,

Или не съм се изразил правилно, или не си ме разбрал.
По време на лекцията предложих алтернативен вариант, в който не зависим от подредбата на елементите.

Разбира се, този подход има малка лимитация, че не трябва да използваме обекти като Hero и Whеel извън нашия Engine.
Смятам, че не е края на света, ако имаме тази лимитация.
Все пак тези обекти биха пренадлежали на Game, а не на Engine.

Моето предложение:

class Engine {
  Engine(ManagerHandler& managerHandler) : mManagerHandler(managerHandler){

...
private:
  //engine only uses the object. It is being created, initialized, 
  //deinitialized and destroyed elsewhere
  ManagerHandler& mManagerHandler;
};

Товага ще създадем и унищоваме ManagerHandler външно за Engine-а като гарантираме, че той (и респективно глобалните Singletons) ще са живи, докато имаме Engine (и по време не целия му destruction call stack).

void runApplication() {
  ManagerHandler managerHandler;
  managerHandler.init();

  std::unique_ptr<Engine> engine = std::make_unique<Engine>(managerHandler);
  engine->init();
  engine->start();

  engine->deinit();
  engine.reset(); //to explicitly invoke the Engine destructor

  managerHandler.deinit(); //or his destructor
}

Поздрави

10/11/2021 19:22:34

MartinBG 4803 Точки

Привет, Живко,

Благодаря за уточнението и най-вече за кода, защото има много тънък момент в него! :)

Опитах това, което предлагаш и наистина работи, като разковничето е в това, че ръчно менажираме живота на Engine обекта и го "убиваме" преди да се излезе от runApplication() (с engine.reset() или delete engine, ако използваме raw pointer) - без този ред проблемът си остава.

Поздрави!

10/11/2021 20:38:21