C++ classのデストラクタでの失敗

Question

前提・実現したいこと

C++ classのおそらくデストラクタでbreak pointが発生します.
デストラクタの記述に問題があるのでしょうか?オブジェクトの生成時に問題があるのでしょうか?
Cしか弄ったことがないのでその他指摘がありましたらお願いします.

該当のソースコード

#define UNICODE

#include <iostream>
#include <Windows.h>

class AllocSample {
private:
	LPWSTR str_1;
	LPWSTR str_2;

public:
	AllocSample(LPCWSTR = nullptr, LPCWSTR = nullptr);
	AllocSample(const::AllocSample&);

	int SetStr_1(LPCWSTR);
	int SetStr_2(LPCWSTR);
	int ShowStr();

	~AllocSample();
};

int main() {

	AllocSample a;
	a.SetStr_1(L"AAAAAAAAAAAAAAAAAA");
	a.SetStr_2(L"BBBBBBBBBBBBBB");

	a.ShowStr();

	AllocSample b(L"aaa", L"bbbbb");//initialize with arguments
	b.ShowStr();

	AllocSample c = a;//initialize by copy
	c.ShowStr();
	

	return 0;
}

AllocSample::AllocSample(LPCWSTR arg_1, LPCWSTR arg_2) :str_1(const_cast<LPWSTR>(arg_1)), str_2(const_cast<LPWSTR>(arg_2)) {};
AllocSample::AllocSample(const::AllocSample& obj) :str_1(obj.str_1), str_2(obj.str_2) {};
AllocSample::~AllocSample() {
	delete[] str_1;
	delete[] str_2;
	std::wcout << L"Destructor Called" << std::endl;
}

int AllocSample::SetStr_1(LPCWSTR lp) {
	int length = lstrlenW(lp) + 1;

	delete[] str_1;
	str_1 = new wchar_t[length];

	for (int i = 0; i < length; i++) {
		str_1[i] = lp[i];
	}

	return 0;
}

/*  same as AllocSample::SetStr_1 */
int AllocSample::SetStr_2(LPCWSTR lp) {
	int length = lstrlenW(lp) + 1;

	delete[] str_2;
	str_2 = new wchar_t[length];

	for (int i = 0; i < length; i++) {
		str_2[i] = lp[i];
	}

	return 0;
}

int AllocSample::ShowStr() {
	if (str_1 != nullptr)std::wcout << L"string 1 : " << str_1 << std::endl;
	else std::wcout << L"-NO DATA-" << std::endl;
	if (str_2 != nullptr)std::wcout << L"string 2 : " << str_2 << std::endl;
	else std::wcout << L"-NO DATA-" << std::endl;

	return 0;
}

補足情報

Windows10 Pro
VisualStudio2019 Community

P.S. 解決方法

AllocSample::AllocSample(LPCWSTR arg_1, LPCWSTR arg_2) :str_1(nullptr), str_2(nullptr) 
{
	if (arg_1 != nullptr) {
		int length = lstrlenW(arg_1) + 1;
		str_1 = new wchar_t[length];

		for (int i = 0; i < length; i++) {
			str_1[i] = arg_1[i];
		}
	}

	if (arg_2 != nullptr) {
		int length2 = lstrlenW(arg_2) + 1;
		str_2 = new wchar_t[length2];

		for (int i = 0; i < length2; i++) {
			str_2[i] = arg_2[i];
		}
	}
};

AllocSample::AllocSample(const::AllocSample& obj) :str_1(nullptr), str_2(nullptr)
{
	int length = lstrlenW(obj.str_1) + 1;
	this->str_1 = new wchar_t[length];
	for (int i = 0; i < length; i++) {
		str_1[i] = obj.str_1[i];
	}

	int length2 = lstrlenW(obj.str_2) + 1;
	this->str_2 = new wchar_t[length2];
	for (int i = 0; i < length2; i++) {
		str_2[i] = obj.str_2[i];
	}
};

Constructor, Copy Constructorを以上のように修正しました.

Comments

[cateye]

Win関係の型定義(LPCWSTR等)は分からないのですが、メモリ解放時は、アドレスが有効かどうか確認しましょう。・・・この辺はCでも同じだと思いますが？

Answer 1

こんにちは。

実際にデータがどのように流れているのか考えるのも手です。

コンストラクタの仮引数arg_1, arg_2で受け取ったポインタをポインタのままstr_1, str_2へ設定し、デストラクタでdelete[]しています。
ということは、コンストラクタへ渡せる実引数は、delete[]可能なものに限られます。しかし、文字定数を渡しています。
このケースでは運のいいことにC++はエラーを報告してくるのですが、せっかくのエラーをconst_castで無理っと回避しています。実は、C++をそれなりにマスターするまではキャストはなるべく使わない方が良いです。特にconst_castとreinterpret_castはそのメリットを理解していないのに使ってしまうと、なかなか痛い目にあいやすいです。

また、AllocSample c = a;でコピー・コンストラクタを呼び出していますが、コピー・コンストラクタでは、str_1とstr_2をコピーしているだけなのでポインタの指す先はコピーされていません。
cをデストラクトする際に、str_1とstr_2が指す先が開放されます。それはaに設定されていたものと同じですので、続けてaをデストラクトする際に、同じものを再度開放しようとして未定義動作となります。

Answer 2

• AllocSample(LPCWSTR, LPCWSTR)が使われた場合：指定された領域をデストラクタで勝手にdelete[]しようとするので大問題．（素直にSetStr_1()とSetStr_2()を呼べば良いと思う）
• コピーコンストラクタを使った場合：デストラクタでコピー元が使っていた領域を勝手にdelete[]しようとするので大問題．同じ領域に多重にdelete[]するのはダメ．
• （今は記述が無いだけかもしれないけども）operator=も面倒みてあげないとコピーコンストラクタと同じ結果になるので注意が必要．

あと，SetStr_1()とSetStr_2()では，引数のnullチェックを入れると良いかと．

Answer 3

1. const_cast を使うときは注意しましょう。コンパイルは通るようになるかもしれませんが、constへのdeleteができるようになるわけではありません。
2. コピーコンストラクタで、フィールドのポインタをコピーしてもポインタの差し示すオブジェクトがコピーされるわけではありません。