shared_ptrを用いた状態管理

Question

C++のスマートポインタ使った状態管理についての質問です。

自身のポインタを戻り値としたClass* Class::Update()関数で状態を管理するプログラムをshared_ptr<Class>を用いて書きたいのですが、
shared_ptr<Class> class class.reset(class->Update())という書き方だとアクセス違反となってしまいます。
この原因が分かりません。スマートポインタの仕様でclass変数が解放されているのでしょうか？
どうかご教授お願い致します。
下記の例のプログラムは、CSMA/CAのシミュレーションを作っています。
以下 struct Channelとstruct ChannelStateを使ったプログラム例(一部割合)

Channel.h

/*通信路クラス*/
struct Channel {
public:
	static shared_ptr<Channel> channel; /*シングルトン*/
	static list<shared_ptr<PassChannel>> TerminalList; /*端末情報*/
	static list<shared_ptr<Packet>> packets; /*通信路に送られたパケット*/
	Channel();
	void Update();
private:
	shared_ptr<ChannelState> state; //通信路の状態
};

Channel.cpp

void Channel::Update() {
	state.reset(state->Update()); //アクセス違反
	return;
}

ChannelState.h

/*通信路状態の基底クラス*/
struct ChannelState {
public:
	ChannelState();
	virtual ChannelState* Update();

};

/*パケット送信可能状態*/
struct Idle:public ChannelState {
public:
	Idle();
	ChannelState* Update() override;
}; 
/*パケット送信中状態*/
struct Busy:public ChannelState {
public:
	Busy();
	ChannelState* Update() override;
private:
	shared_ptr<Packet> packet;
	int Counter;

};

/*衝突発生*/
struct Collision :public ChannelState {
public:
	Collision();
	ChannelState* Update() override;
};

ChannelState.cpp

ChannelState* Idle::Update() {
	if ((int)Channel::packets.size()==1) {
		return new Busy();
	}
	else if ((int)Channel::packets.size()>1) {
		return new Collision();
	}
	return this;
}

ChannelState* Busy::Update() {
	Counter++;
	if (Counter == packet->transferTime) {
		list<shared_ptr<PassChannel>>::iterator itr;
		itr = Channel::TerminalList.begin();
		while ((*itr)->GetID() != packet->distination) {
			itr++;
		}
		(*itr)->ReceivePacket(packet.get());
		return new Idle(true);
	}

	return this;
}

ChannelState* Collision::Update() {
	if ((int)Channel::packets.size()==1) {
		return new Busy();
	}
	else if (Channel::packets.size() >= 1) {
		return new Collision();
	}

	return this;
}

Answer 1

生ポインタから std::shared_ptr を作った時点が共有状態の管理の開始であると考えるとわかりやすいです。

たとえば以下のようなコードは問題があるコードだというのがわかるでしょうか。

#include <memory>
#include <iostream>

int main(void) {
  int *p = new int(1);
  std::shared_ptr<int> sp1(p);
  std::shared_ptr<int> sp2(p);
    
  std::cout << sp1.use_count() << std::endl;
  std::cout << sp2.use_count() << std::endl;

  return 0;
}

sp1 を作った時に「よし、 p の管理を開始するぞ！」となっていて、それとは別に sp2 を作った時にも「よし、 p の管理を開始するぞ！」となるので sp1 と sp2 はお互いに同じものを管理していることを知らないのです。 ですから一方の管理においてカウンタが 0 になってオブジェクトを解体するともう一方を通じてオブジェクトにアクセスすると違反になってしまいます。

同様のことが reset についても言えます。 std::shared_ptr オブジェクトが既に管理対象としているポインタと新たに与えるポインタが同一である場合には同じポインタについての管理が二重になってしまいます。

解決方法としては以下のみっつが思いつきます。

• 専用のポインタ管理機構を作る
• Update が常に (新しいオブジェクトを生成して) 新しいポインタを返す
• Update が以前と同じポインタを返してきた場合には reset しない

Comments

[gRivXOzYyYyDl1W]

ご回答ありがとうございます。
shared_ptrの仕様を誤解していました。
おかげさまで無事解決しました。
ありがとうございました。

Answer 2

virtual ChannelState* Update() は、自分自身か、新しいChannelStateクラスのインスタンスを返すメソッド。で合っていますか？

前提ですが、なるべく左辺値参照とunique_ptrを使うべきで、shared_ptrは避けるべきです。
そのリソースを開放する義務のある人が誰なのか、わかりにくくなる為です。
（shared_ptrしか見当たらないので、念の為）

アクセス違反を起こしている箇所ですが、思い浮かんだのは２つです

・別の場所の実装でリリースされて state が空になっている（この場合質問文から判断できないです）
・メモリリークを起こしている（この場合アクセス違反にはならないと思いますが）

メモリリークを起こしている、についてのみ説明します。
該当箇所を以下のように分解してみます。

auto p = state.get();
state.reset();
auto new_p = p->Update();
state.reset(new_p);

p->Update() で新しいインスタンスを返すとき、古いインスタンス(p)は誰にも解放されず残り続けることが分かると思います。resetは所有権を放棄するだけで、解放しない為です。

解決策としては、Collision::Updateが常に新しいインスタンスを返すようにすることです。
例えば、以下のようにする、等です。

std::unique_ptr<ChannelState> Idle::Update() {
    if ((int)Channel::packets.size()==1) {
        return std::make_unique<Busy>();
    }
    else if ((int)Channel::packets.size()>1) {
        return std::make_unique<Collision>();
    }
    return std::make_unique<Idle>(*this);
}

Comments

[gRivXOzYyYyDl1W]

ご回答ありがとうございます。
shared_ptrの仕様を誤解していました。
今後はunique_ptrを意識してプログラミングします。
ありがとうございました。