VC++(Doc-View使用)にて以下のようなCArrayを含むクラスCLを定義し、
CListにて管理しようと考えています。以下が動作確認用に作成したソースです。
そこで、
①以下を実行後にVSにメモリーリークがあると指摘されます。
(おそらく①のCL::~CL()デストラクタ内だと思われます)
理由と対処方法を教えて頂けないでしょうか。
②CL::~CL()内がdelete x;の場合、ここでアクセス違反強制終了となります。
CArrayクラスですから削除するメンバRemoveAllを使えと言ってしまえば
それまでですが、できればnew使ったのにdeleteできない理由を教えて
頂けないでしょうか。
(連続投稿:CArray CListのメモリ開放に関して(2)もご覧願います)
class CL //自分で定義したクラス
{
public:
int a;
CArray <double, double> *x;
public:
CL::CL(){ //コンストラクタ
a = 0; //適当な値で初期化
x = new CArray <double, double>;
for (int i = 0; i < 10; i++){
x->Add(i); //適当な値で初期化
}
}
CL::~CL(){ //デストラクタ
x->RemoveAll(); //CArrayの要素を削除 ①ここがリークの原因??
//delete x; ②deleteだとここでアクセス違反強制終了
}
};
class CXXXDoc : public CDocument
{
public:
CList <CL, CL& > m_list; //CListの宣言
};
CXXXDoc::~CtestDoc()
{
if (!m_list.IsEmpty()){
m_list.RemoveAll(); //リスト要素の削除(問題なし?)
}
}
void CXXXView::OnDraw(CDC* /*pDC*/) ←適当な関数
{
CL temp;
pDoc->m_list.AddTail(temp); //リストに適当な1要素を追加
}
・CL はメンバ変数として [ポインタ] である x を所有している
・コピーコンストラクタや代入演算子を自作していないので shallow copy のみ
なので CL のデストラクタで delete すると、二重 delete になる。
例題のように delete せずに RemoveAll だけだとリーク+データ損傷する。
結局のところ「なにがしたいのか」次第なわけだが、上記コードがすべてならば、
言語仕様どおり当然の動作を行っているわけで・・・
・CArray* x をメンバーに持つのではなくて CArray 自体を持つように修正する
→全 CL インスタンスごとに別の CArray を保持する
(もしかしたらメモリの無駄かも?実行速度の無駄かも?)
・CArray を複写すべきか否かの検討をきっちり行い、複写なり共有なりさせる
=コピーコンストラクタ・代入演算子を正しく実装する
→ deep copy するくらいなら1案のほうがマシ
→ shared_ptr などで共有するには基幹設計から見直し
CArray は CObject 派生であり CObject::CObject や CObject::operator= は private
つまりコピー不可、代入不可ってことか・・・
ならば選択肢はかなり限られ
・ deep copy/deep assignment を手作りするか
・コピーしなくて済むよう共有化を考えるか
(シリアライズで Load したときどうなるか慎重な配慮が必要)
・腐れコレクションである CArray などポイして std::vector 化するか
(シリアライズ部は自作しなきゃならない)
・そもそも Doc のデータ構造自体の見直しを行うか
あたりになりそう。
書かれている内容を見て疑問に思ったのは、
なぜCArrayをポインタで実装しているのかと言う話。
CListへAddする時にエラーが出ていてそれを解決しようとした?
実際にコンパイルをして見たわけでは無いので想像で書いていますが。
tetrapodさんが指摘しているように自作のクラスに代入演算子や
コピーコンストラクタの実装が無いのでメンバー変数をそのままコピーする
実装になっていると言うのは理解できるでしょうか。
同じアドレスを指すポインタがAddする為に作成したtempと
CList内に存在するインスタンスの二箇所で保持された状態になり、
tempはローカル変数なのでスコープが外れた時点でデストラクタが
動き、CList内のインスタンスが持つポインタの値は既に解放済みの
アドレスを指している状態になっています。
クラスは各インスタンスで独立して動けるように作成されていないといけないので
今の作りでは同じアドレスをさすポインタが代入やコピーの度に増えていってしまうので
まずいです。
素直に実装するならポインタではなく、実体で持つようにして
代入演算子とコピーコンストラクタを実装し、
その中でArrayの内容を全てコピーするようにプログラムを書く方がわかりやすいと思い
ます。
わざわざポインタで実装しても問題を複雑にするだけでメリットが感じられません。
CArrayを複数のインスタンス間で共有したいという話ならポインタでの実装も
わからなくはないですけれど。
なんか、暗黙にコピーされちゃってのかもしれませんねぇ。
tetrapodさん、PATIOさんの回答どおりなので、自分は
このような場合のデバッグのコツを書いておきます。
デフォルトコンストラクタ、コピーコンストラクタ、コピーオペレータ等
は明示的にコードしなくても使われてしまいます。
templateを含む外部ライブラリ(MFC、STL)を使う場合は、用心してこれらが
暗黙に使われた場合にエラーが出るようにコードしとくと便利です。
本件の場合は
private: CL(const CL& ex){}
private: CL& opraator =(const CL& ex){}
などを追加してみてください。
エラーが出るようならこれらが使われてるわけですね。
その場合は当該メンバの中身を正しくコードしなおすか、
これらが使われないように仕組み全体を変更します。
>なんか、暗黙にコピーされちゃってのかもしれませんねぇ。
確か「Effective C++」にも書かれてたと思うので、
既定の動作としては多分間違い無いと思います。
デフォルトのコピーコンストラクタと代入演算子の実装は
メンバー変数をそっくりコピーする実装になっていたはずです。
C言語の構造体の動作に合わせる為じゃないかと思っています。
VC++が何処まで従っているんだという話はあると思いますけれど、
今までの経験上は従った動作になっていたと思います。
で、そっくりコピーする実装になっているのに
CArrayの代入演算子はprivateなので使えないと言う事で
多分、ビルド時にエラーになってしまい、
ビルドエラーを何とかする為にポインタにしてしまったのかなと。
ビルドエラーを解決したいのはわかるのですけれど、
動作が変わってしまうような修正は駄目だと思います。
これはビルドエラーの根本原因が分からない状態で修正をしてしまっている為ですね。
ビルドエラーを解決する時は、根本の原因を理解した上で修正するようにしないと
同様の問題に突き当たると思うのできちんと原因の調査をした方が良いと思います。
皆様>
早速のご回答有難う御座います。
皆様がご指摘の内容を一つずつ参考書・CArrayの定義部等を読みながら理解致しました
(本で読みましたが身についてなかったです・・・)。起こっている現象は「C++クラス
と継承完全制覇」という本で書かれているようにコンストラクタ・デストラクタの呼び出
されるタイミング、メンバ変数にポインタを含む場合、コンストラクタ・デストラクタに
new, deleteを含む場合、一つのインスタンスを複数のポインタが指す状態が作成された
場合、コピーコンストラクタ、代入演算子の多重定義で解説されている内容でした。たぶ
ん理解できました。有難う御座います。
たぶん今回の場合CArray自体をメンバに持てば良いと思います。
特にCArrayをインスタンス間で共有する必要はなさそうです。
(下部ご覧ください)
PATIO様 2011/11/07(月) 16:51:04>
>CArrayの代入演算子はprivateなので使えないと言う事で
>多分、ビルド時にエラーになってしまい、
>これはビルドエラーの根本原因が分からない状態で修正をしてしまっている為ですね。
ご指摘のとおりです。
教科書の例題以外では、実はこれがC++(VC++)で最初に作成したコードでした。(上記
は質問のための抜粋です)
一発目なのでとにかく作ってみようと思いまして(但し昔はC使ってました)。
練習としてグラフを描画するソフトを作ってみようかと思いました。
1つのグラフ用データを1つのクラスとして、そのメンバに大量の変数並びにCArrayにて
前述変数群から計算した結果をx,yとして保存し、クラス郡をCListにて管理しようと思い
ます。クラス郡の各変数はCtrlListに表示し、プロパティシートを使って各変数を入力修
正し、それをCList、CtrlListに反映し管理したいと思います。更にpictureControlに各
クラスのCArrayに保存したx,yをプロットしたいと考えています。データ保存・読み込み
も考慮します。とりあえず動くまではいったのですがデータ管理で最初のバグ作っていま
した・・・(汗)。
ちょっとあれやこれやと内容が入ってますが、使っている部品の組み合わせが悪くてもい
ろいろ練習してみようかと思いました。
というのが現状です。
仲澤@失業者様 2011/11/07(月) 10:41:45>
PATIO様 2011/11/07(月) 16:46:27>
tetrapod 2011/11/07(月) 09:09:14>
初なのでちょっと鈍いかもしれませんが(自信がないので)
class AFX_NOVTABLE CObject (←これかどうかはわからないが
afx.hより抜粋、CArrayの親?)
{
private:
CObject(const CObject& objectSrc); // no implementation
void operator=(const CObject& objectSrc); // no implementation
・・・
}
↓
>CArray は CObject 派生であり CObject::CObject や CObject::operator= >は
privateつまりコピー不可、代入不可ってことか・・・
↓
>なんか、暗黙にコピーされちゃってのかもしれませんねぇ。
↓
>確か「Effective C++」にも書かれてたと思うので、
>デフォルトのコピーコンストラクタと代入演算子の実装は
>メンバー変数をそっくりコピーする実装になっていたはずです。
↓
CArray *xとした場合は、CArrayの代入演算子はprivateなので使えないので、只のポイン
タ変数*xなので暗黙のうちにメンバー変数をそっくりコピーすると解釈されて、コンパイ
ラは通る(った)。意図しているように動作するかどうかは不明。この際、代入=が実体
のないポインタを作らないように適切にoperator=を定義しなければならない。またデス
トラクタにてdelete xすることを考慮してコピーコンストラクタを適切に定義しなければ
ならない。
CArray xとした場合は、CArrayの代入演算子はprivateなので使えないし、暗黙にコピー
すらできなかった?(要素あるし)のでコンパイラすら通らなかった。そもそも
operator=、コピーコンストラクタを定義しなければならない。
で大枠合ってますでしょうか??
仲澤@失業者様 2011/11/07(月) 10:41:45>
>templateを含む外部ライブラリ(MFC、STL)を使う場合は、用心してこれらが
>暗黙に使われた場合にエラーが出るようにコードしとくと便利です。
>本件の場合は
>private: CL(const CL& ex){}
>private: CL& opraator =(const CL& ex){}
>などを追加してみてください。
>エラーが出るようならこれらが使われてるわけですね。
>その場合は当該メンバの中身を正しくコードしなおすか、
>これらが使われないように仕組み全体を変更します。
最初のコードの宣言部に上記ご指摘を追加したところ、
「'CL::operator =' : private メンバー (クラス 'CL' で宣言されている) にアクセス
できません。」
と返ってきました。
この部分デバッグのコツということですが(エラーがある・なしも含めて)
理解できてないので再度解説お願いできないでしょうか。
何卒宜しくお願い致します。
> operator=、コピーコンストラクタを定義しなければならない。
大枠あっている、と思っていいだろうね。
# 文に主語=クラス名がないので本当にあっているか判断不能。
提示 CL は new/delete するポインタメンバを保持しているクラスなわけで、
典型的 [コピーコンストラクタ、代入演算子を自作する必要がある] 例。
現 CL の CArray *x; を CArray x; に書き換えても CArray がコピー代入不可なので
CArray に対するコピー・代入を CL 中で自前実装しなければならない。
# std::vector ならコピー・代入できるので簡単(シリアライズは自作する必要あり)
CArray を使うならコピー・代入のコード
std::vector を使うならシリアライズのコード
の実装例を俺たちが書くのは簡単なのだが、それでは演習にならないので
eizo 氏が自分で実装してみると良いと思うぞ。
>この部分デバッグのコツということですが(エラーがある・なしも含めて)
つまり「CLクラスのコピーオペレータが、MFCのテンプレートで使われていた」
のですね。以前は未定義だったので、C++言語の仕様にのっとって、
暗黙のpublicなコピーオペレータが使われていたのですが、今回は、privateに
明示的に定義されたので使えなくなり、エラーが出るわけですね。
ちなみに、暗黙の方が何をしてくれるかというと、全メンバの単純なコピー
のみです。従って、xについては、そのアドレスがコピーされてしまいます(泣)。
さて、ではCLのコピーオペレータは、どのように振舞うべきでしょうか。
コピーなので、関数を除く全メンバが同じ内容になるべきですよね。
int a;はそのままコピーするとして、配列xの内容が同じとは、どうあるべきか
が重要で、当然アドレスであるxが一致してもしかたありません(vv;)。
つまり一般には、xの配列の中身が同じになるべきではないでしょうか。
つまり、本件のCLクラスの場合
1.コピーする前に現在の自分のxの内容を破棄する。(これを良く忘れる orz.)
2.引数のインスタンスのxの中身を、自分のxに全てコピーする。
ですね。
もちろん正しくコードした当該のコピーオペレータは、
public:にしとかないと誰も使えません(vv;)。
皆様>
ご返答ありがとうございます。
tetrapod様 2011/11/08(火) 07:19:26>
1.CArray *x; にてコピーコンストラクタ、代入演算子の練習
2.CArray x; へプログラム改造(兼コピーコンストラクタ、代入演算子の練習)
以上今週中
3.STLへの練習と移行(来週から)
→CListだとループ中で検索してpos取るのにGetPrevしないと
いけないですし、一部戻り型が不便なものもあります。
遥か昔、Cの初歩の練習で作成した線形リストのほうがましな感じです。
で進めていこうと思います。
仲澤@失業者様 2011/11/08(火) 13:21:00>
>前半~中盤部分
ご説明ありがとう御座います。仰ることが理解できました。
>後半部分
xが指す動的オブジェクトを一旦開放してから自分のxにコピーします。
ということで一旦このレスをクローズさせて頂きたいと思います。
また別の質問をするかもしれませんがご指導宜しくお願い致します。