Singletonパターンの解放のタイミング（？）

[かめり]

現在以下のような、Singletonパターンを使ったクラス GameStateMan　を使ってゲーム全体を管理する仕組みを作っています

コード:

//ゲーム状態識別子
enum GameState_t
{
	GAME_STATE_OPENING,
	GAME_STATE_TITLE,
	GAME_STATE_MAINGAME,
	GAME_STATE_ENDING,
};

      
//ゲーム状態管理クラス
class GameStateMan
{
protected:
	GameState_t m_gameState;
public:
             //インスタンス
	static GameStateMan *Instance()
            {
            　　　　　　 static GameStateMan instance;
                          return &instance;
　　　　　　}       			
	//状態セット				
	void SetState( GameState_t setState )							
　　　　　　{
　　　　　　　　　　　　m_gameState = setState;
　　　　　　}
             //状態取得
	GameState_t GetState()const								
　　　　　　{
　　　　　　　　　　　　return m_gameState;
　　　　　　}
private:
　　　　　　//コンストラクタ
　　　　　　GameStateMan( GameState_t initState = GAME_STATE_OPENING )			
　　　　　　{
　　　　　　　　　　　　m_gameState = initState;
　　　　　　}
	GameStateMan(const GameStateMan& r){}
	GameStateMan& operator=(const GameStateMan& r){}
};

int main()
{
	//メインループ
	while(1)
	{
		switch( GameStateMan::Instance()->GetState() )
		{
		//Opening(),Title(),MainGame(),Ending()はそれぞれGameState_t型の値を返します
		case GAME_STATE_OPENING:   GameStateMan::Instance()->SetState( Opening() );
		case GAME_STATE_TITLE:        GameStateMan::Instance()->SetState( Title() );
		case GAME_STATE_MAINGAME: GameStateMan::Instance()->SetState( MainGame() );
		case GAME_STATE_ENDING:     GameStateMan::Instance()->SetState( Ending() );
		}
	}
	return 1;
}

実行中は問題ないのですが、ループを抜けて終了する際、
”_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)”といったエラーが表示されます。
ググッて見たところメモリが確保されていない領域をdeleteした時に出るエラーらしい事が分かりましたが、
修正の仕方が分からず困っています。
お手数ですが、どなたか修正の仕方を教えていただければと思います。
また、Singletonパターンの他にも唯一つのインスタンスを作成する方法があればそちらもお願いいたします

[h2so5]

動く状態のコードを貼ってもらわないとアドバイスしようがないです。

このコードだとループをから抜けられないですし、
switch内にbreakがないので常にGAME_STATE_ENDINGが設定されてしまいます。

あとこの設計だとSingletonパターンの利点があんまり無いですね。
グローバル変数を使ったほうがいいかと。

[かめり]

h2so5さん、返信ありがとうございます
確かに省略しすぎて何じゃこりゃなコードになっていますね、すいません。
グローバル変数でよいのでは？とのことでしたが
GameStateManのSetState()に後々処理を追加していく予定なのでクラスで設計しています。
以下書き直したコードになります（GameStateManの記述は割愛させていただきました）。

コード:

//ループで必ず行う３大処理
int ProcessLoop(){
    if(ProcessMessage()!=0)return -1;//プロセス処理がエラーなら-1を返す
    if(ClearDrawScreen()!=0)return -1;//画面クリア処理がエラーなら-1を返す
    GetHitKeyStateAll_2();//現在のキー入力処理を行う
    GetHitPadStateAll();  //現在のパッド入力処理を行う
    return 0;
}

int WINAPI WinMain(HINSTANCE hInstance,HINSTANCE hPrevInstance,LPSTR lpCmdLine,int nCmdShow){

	if( DxLib_Init() == -1 )// ＤＸライブラリ初期化処理
		return -1;// エラーが起きたら直ちに終了


	while ( ProcessLoop() == 0 )
	{

		//エスケープが入力ていたら
		if ( CheckStateKey ( KEY_INPUT_ESCAPE ) == 1 )
			break;
		//  ゲーム全体の状態で振り分ける。
		switch( GameStateMan::Instance()->GetState() )
		{
		//Opening(),Title(),MainGame(),Ending()はそれぞれGameState_t型の値を返す
		case GAME_STATE_OPENING:							//オープニング
			GameStateMan::Instance()->SetState( Opening() )		;	break;
		case GAME_STATE_TITLE:								//タイトル画面
			GameStateMan::Instance()->SetState( Title() )		;	break;
		case GAME_STATE_MAINGAME:							//ゲーム本編
			GameStateMan::Instance()->SetState( MainGame() )	;	break;
		case GAME_STATE_ENDING:								//エンディング
			GameStateMan::Instance()->SetState( Ending() )		;	break;
		}
	}


	DxLib_End();//ＤＸライブラリ終了処理

	return 0;
}

よろしくお願いいたします。

[とっち]

h2so5さんもおっしゃられてますがこれだとSingletonパターン
じゃなくていい気がします。

コード:

int WINAPI WinMain(略){
	if( DxLib_Init() == -1 )// ＤＸライブラリ初期化処理
		return -1;// エラーが起きたら直ちに終了

	GameStateMan game=new GameStateMan();
	while(ProcessLoop() == 0){
		switch(game->GetState()){
			略
		}
	}

	// メインループを向けると処理終了ということなので終了処理
	delete game;
	DxLib_End();//ＤＸライブラリ終了処理
	return 0;
}

これでいいのでは？

[かめり]

とっち さん応答ありがとうございます。
なるほど、無理にSingletonパターンを使わなくてもこのケースの場合
素直にnewして使った方が今回のようなややこしいエラーも起きませんし良いですね。
（現在のコードではOpening()やTitle()等関数群が　return GameState::Instance()->GetState;　と値を返していたので
　そちらの方に気を使っていました）
ですが今回のエラーはどうして起こったのでしょうか（しつこいなおい）？
WINAPI WinMain(HINSTANCE hInstance,HINSTANCE hPrevInstance,LPSTR lpCmdLine,int nCmdShow)が
return　するとき（プログラム終了時）にstaticで宣言されたインスタンスが解放されると思うのですが
そのほかにdeleteもfree模していないのに何故今回のようなエラーが出たのでしょう？

[h2so5]

＞とっちさん
GameStateManを動的確保する必要はあるのでしょうか？

＞かめり
正直、このコードだと余計な部分が多くてよく分からないです。
もうちょっとコンパクトなコードで再現できるようにするか、プロジェクトごとアップロードしてください。

[かめり]

＞h2so5さん
firestrageにプロジェクトのzipをあげさせていただきました
http://firestorage.jp/download/0fe6b40a ... 68f8a7b507
よろしければこちらからエラーの状態の方を確認してください。
制作環境はVisual C++ 2010 Expressです

[かめり]

補足です。
現在組み立て中のプロジェクトなので色々と適当な部分がありますが、
GameState.h　と　GameStateMan.cpp、　Continue.cpp Demo.cpp Ending.cpp MainGame.cpp NameEntry.cpp Opening.cpp Ranking.cpp Result.cpp Title.cpp
Main.cpp
が今回の問題の部分になっているコードです。
よろしくお願いします！

[h2so5]

プロジェクトを拝見しました。
こちらで実行したところ、StateManObject のメンバの m_cnt のデストラクタで assertion failed が発生しています。

エラー箇所の切り分けができていないので、デバッグの仕方をもっと勉強したほうが良いと思います。
エラーの起きている箇所が明らかに GameStateMan と関係ない箇所ですし、どうしてその部分が原因だと思ったのか謎です。
（ヒープの状態によって、そちらでは違う結果になっていたのかもしれませんが...）

StateManObject の周りを調べたところ明らかにおかしな部分があり、
例えば StateManObject.cpp の 91行目

コード:

		m_cnt.get()[m_nowState] = 0;			//  フレームカウントを初期化。

ブレークポイントを入れると分かりますが、m_nowState が負になっているので、ヒープが破壊されます。

m_cnt は shared_ptr<int> より vector<int> が適切だと思います。
全体的にスマートポインタを濫用している感じがするので、使い所を見極めたほうが良いです。


[shared_ptrについて]

今回のエラーとは関係ないと思いますが、shared_ptr はデフォルトで delete を呼ぶので、配列を入れるとリークします。
配列を使いたい場合はカスタムデリータを設定して delete[] を呼ぶ必要があります。もしくは、unique_ptr<int[]> を使用します。
◯

コード:

    //フレームカウント配列を確保する。
    if( stateNums > 0 )
		m_cnt.reset( new int[stateNums], [](int* ptr){ delete[] ptr; } );

また、このようなコードは冗長すぎます
NULLで初期化したり、いちいちresetを使うのは意味がなく分かりにくいだけです。
☓

コード:

CrashPtr CrashRect::Make( float x, float y, int width, int height )
{
	CrashPtr newCrash = NULL;
	newCrash.reset( new CrashRect( x, y, width, height ) );

	return newCrash;
}

こう書きましょう
◯

コード:

CrashPtr CrashRect::Make( float x, float y, int width, int height )
{
	return make_shared<CrashRect>( x, y, width, height );
}

デストラクタでわざわざresetを書く必要はありません
☓

コード:

//デストラクタ
StateManObject::~StateManObject()
{
	//フレームカウント配列を破棄
	m_cnt.reset();
}

[かめり]

＞ h2so5さん
ぬおおお、ありがとうございます。ご指摘のとおりデバッグはほぼ使えていない状況で
エラーの原因を探るのはほぼ体当たり作業となっていました、StateManObjectは他のプロジェクトで普通に使えていたように見えていたので
そちらに気が回りませんでした。勉強しようと思います。

またshared_ptrについても色々と学ばせていただきました、重ね重ね感謝です。
本当にありがとうございました！