Пытаясь реализовать простой пример State Pattern из книги «Head First Design Patterns», я столкнулся с ситуацией, которая мне кажется странной. Имейте в виду, этот вопрос не о правильной реализации шаблона, а о понимании основного механизма, который ведет к наблюдаемому поведению.
Машина «Gumball_machine» должна иметь несколько возможных состояний (No_quarter_state
, Has_quarter_state
, Sold_out_state
и т. д.), поведение которого делегируется посредством вызовов виртуальных функций во время выполнения. Эти состояния публично наследуются от абстрактного базового класса State
, Gumball_machine
имеет std::unique_ptr<State>
, State
сам класс необработанный указатель на Gumball_machine
(поскольку право собственности не предполагается).
Переход состояния происходит, когда выполняются определенные условия, они происходят посредством выделения нового конкретного класса состояния и передачи права собственности на Gumball_machine
,
(Я опубликую некоторые примеры кода в конце этого поста, так как сначала я хочу «добраться до сути».)
Существует одна ситуация, когда в той же функции после переключения состояний вызывается другая функция:
void Has_quarter_state::turn_crank()
{
std::cout << "You turned...\n";
machine_->state_ = std::make_unique<Sold_state>(machine_);
machine_->dispense(); // Invalid read!
// This works however (don't forget to comment out the above reallocation):
// Gumball_machine* ptr{machine_};
// machine_->state_ = std::make_unique<Sold_state>(machine_);
// ptr->dispense();
}
с machine_
быть указателем на Gumball_machine
, а также state_
быть std::unique_ptr<State>
в конкретное состояние, Has_quarter_state
,
Если я объявлю временный указатель ptr
и вызвать Gumball_machine::dispense()
, нет проблем. Однако, если я просто позвоню machine_->dispense()
, valgrind покажет недопустимое чтение (сообщение об ошибке будет показано ниже).
И это я не очень понимаю. ptr
а также machine_
следует относиться к тому же Gumball_machine
экземпляр, который не должен быть уничтожен до конца программы. Has_quarter_state
(или, точнее, родительский класс «State») имеет только необработанный указатель без владения.
Теперь, когда я думаю об этом, это, вероятно, потому что unique_ptr
— сброс приведет к тому, что память, занятая Has_quarter_state
экземпляр должен быть освобожден. Это, вероятно, будет означать, что любое последующее действие, то есть вызов функции Gumball_machine::dispense()
, приведет к неопределенному поведению. Это предположение верно?
Если адрес памяти (&memory_ == &ptr
) не меняется, почему это имеет значение, если я позвоню ptr->dispense()
или же machine_->dispense()
?
Я чувствую, что есть некоторые тонкости управления памятью, которые я до сих пор не понимаю. Надеюсь, вы сможете помочь мне разобраться.
Ниже я опубликую код для воспроизведения этого («неправильная» версия), и сообщение об ошибке, которое выдает мне valgrind (используя --leak-check=full
, --leak-kinds=all
).
Код компилируется через clang++ -std=c++14 -stdlib=libc++
используя clang 3.6.0
Теперь для фактического кода (значительно уменьшен, чтобы быть более минимальным примером):
Gumball_machine.hpp:
#ifndef CLASS_GUMBALL_MACHINE_HPP_
#define CLASS_GUMBALL_MACHINE_HPP_
#include <memory>
class State;
class Gumball_machine
{
friend class Has_quarter_state;
friend class Sold_state;
public:
Gumball_machine();
~Gumball_machine();
void turn_crank();
private:
void dispense();
private:
std::unique_ptr<State> state_;
};
#endif
Gumball_machine.cpp:
#include "Gumball_machine.hpp"
#include "Has_quarter_state.hpp"
Gumball_machine::Gumball_machine() : state_{std::make_unique<Has_quarter_state>(this)} {}
Gumball_machine::~Gumball_machine() {}
void Gumball_machine::turn_crank() { state_->turn_crank(); }
void Gumball_machine::dispense() { state_->dispense(); }
State.hpp:
#ifndef CLASS_STATE_HPP_
#define CLASS_STATE_HPP_
class Gumball_machine;
class State
{
public:
explicit State(Gumball_machine* m);
virtual ~State();
virtual void turn_crank() = 0;
virtual void dispense() = 0;
protected:
Gumball_machine* machine_ = nullptr;
};
#endif
State.cpp:
#include "State.hpp"State::State(Gumball_machine* m) : machine_{m} {}
State::~State() {}
Has_quarter_state.hpp:
#ifndef ClASS_HAS_QUARTER_STATE_HPP_
#define ClASS_HAS_QUARTER_STATE_HPP_
#include "State.hpp"
class Gumball_machine;
class Has_quarter_state : public State
{
public:
explicit Has_quarter_state(Gumball_machine*);
~Has_quarter_state() override;
void turn_crank() override;
void dispense() override;
};
#endif
Has_quarter_state.cpp:
#include "Has_quarter_state.hpp"
#include <iostream>
#include "Gumball_machine.hpp"#include "Sold_state.hpp"
Has_quarter_state::Has_quarter_state(Gumball_machine* m) : State{m} {}
Has_quarter_state::~Has_quarter_state() {}
void Has_quarter_state::turn_crank()
{
std::cout << "You turned...\n";
machine_->state_ = std::make_unique<Sold_state>(machine_);
machine_->dispense(); // Invalid read!
// This works however (don't forget to comment out the above reallocation):
// Gumball_machine* ptr{machine_};
// machine_->state_ = std::make_unique<Sold_state>(machine_);
// ptr->dispense();
}
void Has_quarter_state::dispense()
{
std::cout << "No gumball dispensed\n";
}
Sold_state.hpp:
#ifndef ClASS_SOLD_STATE_HPP_
#define ClASS_SOLD_STATE_HPP_
#include "State.hpp"
class Gumball_machine;
class Sold_state : public State
{
public:
explicit Sold_state(Gumball_machine*);
~Sold_state() override;
void turn_crank() override;
void dispense() override;
};
#endif
Sold_state.cpp:
#include "Sold_state.hpp"
#include <iostream>
#include "Gumball_machine.hpp"#include "Has_quarter_state.hpp"
Sold_state::Sold_state(Gumball_machine* m) : State{m} {}
Sold_state::~Sold_state() {}
void Sold_state::turn_crank()
{
std::cout << "Turning twice doesn't give you another gumball\n";
}
void Sold_state::dispense()
{
std::cout << "A gumball comes rolling out the slot\n";
// machine_->state_.reset(new No_quarter_state{machine_});
machine_->state_ = std::make_unique<Has_quarter_state>(machine_);
}
РЕДАКТИРОВАТЬ: main.cpp
int
main ()
{
Gumball_machine machine;
machine.turn_crank();
return 0;
}
И, наконец, вывод Valgrind:
==12085== Memcheck, a memory error detector
==12085== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==12085== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==12085== Command: ./main
==12085==
==12085== Invalid read of size 8
==12085== at 0x401C61: Has_quarter_state::turn_crank() (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085== by 0x401730: Gumball_machine::turn_crank() (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085== by 0x402FF7: main (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085== Address 0x5e47048 is 8 bytes inside a block of size 16 free'd
==12085== at 0x4C2CE10: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12085== by 0x4017B4: operator delete(void*, unsigned long) (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085== by 0x401858: Has_quarter_state::~Has_quarter_state() (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085== by 0x401B27: Has_quarter_state::turn_crank() (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085== by 0x401730: Gumball_machine::turn_crank() (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085== by 0x402FF7: main (in /home/mbw/Documents/Programmieren/CPP/Design_Patterns/Head_First_Design_Patterns/Chapter10_State_pattern/Example1_Revised/example_for_stackoverflow/main)
==12085==
==12085==
==12085== HEAP SUMMARY:
==12085== in use at exit: 0 bytes in 0 blocks
==12085== total heap usage: 3 allocs, 3 frees, 48 bytes allocated
==12085==
==12085== All heap blocks were freed -- no leaks are possible
==12085==
==12085== For counts of detected and suppressed errors, rerun with: -v
==12085== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Заранее спасибо за вашу помощь!
Проблема в том, что Has_quarter_state
экземпляр, по которому вы звоните turn_crank
уничтожается при замене _machine->state
с новым std::unique_ptr
:
machine_->state_ = std::make_unique<Sold_state>(machine_);
Здесь вы заменяете machine_->state
с новым unique_ptr
который содержит другой объект. Это означает, что ~unique_ptr<State>()
называется прямо перед построением нового unique_ptr
для нового Sold_state
, Но текущий принадлежащий объект уникального указателя является Has_quarter_state
экземпляр, который неявно упоминается this
в методе выполнения.
Тогда что ты делаешь?
ты сделаешь machine_->dispense()
который this->machine_->dispense()
но machine_
является переменной экземпляра объекта, который был только что уничтожен (и для которого вы вызвали текущий метод выполнения), поэтому его значение больше не является допустимым.
Назначение machine_
на временные работы, потому что вы копируете содержимое поля члена объекта, прежде чем уничтожить его. Таким образом, вы все равно можете получить доступ к машине правильно.
Без использования std::unique_ptr
и заставляя каждое состояние управлять своим собственным освобождением, вы видите, что что-то не так, потому что (почти) эквивалентный код (который был бы действительно плохим дизайном) будет следующим:
void Has_quarter_state::turn_crank() {
this->machine_->state_ = new Sold_state();
delete this;
this->machine_->dispense();
}
Теперь вы видите, что сначала вы delete this
затем вы пытаетесь получить доступ к полю, которое является частью освобожденного объекта.
Других решений пока нет …