もくじ
https://tera1707.com/entry/2022/02/06/144447
やりたいこと
あるとき、下記のようなコードを書いた。
class MyClass { private: FILE* fp; public: MyClass() { _wfopen_s(&fp, L"C:\\Users\\masa\\Desktop\\a.txt", L"r"); } ~MyClass() { auto ret = fclose(fp); } void func() {/*fpを使ってファイル読み書き*/} }; std::vector<MyClass> list; void CreateList() { MyClass mc; list.push_back(mc); } int main() { CreateList(); for (int i = 0; i < list.size(); i++) { list[i].func(); } }
- MyClassは、ファイルポインタ経由でファイルを読み書きするクラス。
- func()関数で、ファイルを読み書きする。
- そのクラスを、std::vectorでリストにする。
- そのリストをforで回して、全てのMyClass分だけファイル読み書きする。
ということをしようとした。
当然、
- CreateList()関数の中で、listに入れたMyClassの分だけ、
- MyClassのコンストラクタでファイルが開かれてfpに値が入り、
- listの数だけfunc()が呼ばれて読み書きして、
- mainを抜けるときにMyClassのデストラクタが呼ばれてファイルが閉じられる
と思っていたのだが、結果はそうはならず、
- MyClass mc; の時点でコンストラクタでファイルが開かれるのは予定通りだったのだが、
- CreateList()関数を抜ける時点でMyClassのデストラクタが呼ばれる。
- fcloseでファイルをCloseする。
- 当然その後のファイル読み書きは失敗する
- さらに、mainが終わったタイミングでもう一度MyClassのデストラクタが呼ばれる
- その時は、ファイルのCloseに失敗する。
という動きになった。
調べていて、CreateList()
の中のMyClass mc;
のmcはローカル変数だから、関数から抜けるときにスコープから抜けるので、そこでデストラクタが走るのはなるほどそうかも、となったが、
を調べたい。
調べた結果
結論としては、list.push_back(mc);
で、push_backに渡した時点で「コピー」が行われて、listの中に、mcと同じモノが出来上がっているために、CreateList()
から抜けて、ローカル変数のmcがデストラクトされてしまっても、listの中のmcと同じモノのほうで、func()
関数が呼べていた。
また、最後にもう一度デストラクタが呼ばれるのは、listに持っているコピーの方が、最後でデストラクトされたときのもの。
「コピー」
下記のページによると、コピーが行われると、クラス内の値が全部コピーされるが、int i;
のような値がコピーされるのは問題にならないが、今回のfp
のようにポインターがコピーされると問題になるとのこと。
https://zenn.dev/masahiro_toba/articles/c66972e20d6ee7
つまり今回のパターンだと、
ローカル変数の
mc
が持っているfpが、listにコピーされたmc
のfpと全く同じものになる。つまり、同じファイルを操作することになる。で、デストラクタにfpでファイルを閉じる処理を書いていたので、スコープアウトするときに呼ばれたローカル変数の
mc
のデストラクタで、ファイルが閉じられる。その後、listにコピーされた方のfpでファイルを読み書きやファイルcloseをしようとするが、すでに↑で閉じられているので失敗する
という流れになっていた。
なにがおかしかったのか?
という感じで変な動きをさせてしまっていたのだが、何が悪かったのか?を考えたときに、
- 「1つだけ」作ったつもりのMyClassのインスタンスが、実は2つ作られていた
- さらにそのうちの一つが、知らない間にデストラクトされてしまっていた
- デストラクトの際、知らずにコピーされたポインタを使って資源を勝手に解放されていた
が問題と思った。
で、どう直すのか?を考えたら、
- 知らない間にコピーを作られないようにする
- 1つだけ作成したMyClassを、意図した期間(listの生存期間)だけ生きさせる
ようにすべきかと思う。
対応策(スマートポインタ)
上記の対策をするうえで、今回は「スマートポインタ」を使った。
以下に、対策前のコードと対策後のコードを上げてみる。
対策前コード
#include <windows.h> #include <iostream> #include <memory> #include "vector" static int ctr = 0; class MyClass { private: FILE* fp; int InstanceNumber = 0; public: MyClass() { InstanceNumber = ctr++; wprintf(L"MyClass No.%d \r\n", InstanceNumber); _wfopen_s(&fp, L"C:\\Users\\masa\\Desktop\\a.txt", L"r"); } ~MyClass() { auto ret = fclose(fp); wprintf(L"~MyClass No.%d fclose %s\r\n", InstanceNumber, ret == 0 ? L"success." : L"fail."); } void func() { wchar_t bug[256]; auto err = fgetws(bug, 256, fp); wprintf(L"MyClass No.%d func() fgets = %s\r\n", InstanceNumber, err != NULL ? L"OK" : L"NG"); } }; std::vector<MyClass> list; //std::vector<std::unique_ptr<MyClass>> list; void CreateList() { MyClass mc; list.push_back(mc); } int main() { CreateList(); wprintf(L"after func.\r\n"); for (int i = 0; i < list.size(); i++) { list[i].func(); } }
実行結果
MyClass No.0 ~MyClass No.0 fclose success. after func. MyClass No.0 func() fgets = NG ~MyClass No.0 fclose fail.
対策後コード
#include <windows.h> #include <iostream> #include <memory> #include "vector" static int ctr = 0; class MyClass { private: FILE* fp; int InstanceNumber = 0; public: MyClass() { InstanceNumber = ctr++; wprintf(L"MyClass No.%d \r\n", InstanceNumber); _wfopen_s(&fp, L"C:\\Users\\masa\\Desktop\\a.txt", L"r"); } ~MyClass() { auto ret = fclose(fp); wprintf(L"~MyClass No.%d fclose %s\r\n", InstanceNumber, ret == 0 ? L"success." : L"fail."); } void func() { wchar_t bug[256]; auto err = fgetws(bug, 256, fp); wprintf(L"MyClass No.%d func() fgets = %s\r\n", InstanceNumber, err != NULL ? L"OK" : L"NG"); } }; std::vector<std::unique_ptr<MyClass>> list; // ★ココ void CreateList() { auto mc = std::make_unique<MyClass>();// ★ココ list.push_back(std::move(mc));// ★ココ } int main() { CreateList(); wprintf(L"after func.\r\n"); for (int i = 0; i < list.size(); i++) { list[i].get()->func();// ★ココ } }
実行結果
MyClass No.0 after func. MyClass No.0 func() fgets = OK ~MyClass No.0 fclose success.
対策後は、狙ったタイミングで、デストラクタが呼ばれていた。
ポイント
std::move
によって、ローカル変数として作っていたmcの「所有権」が、list側に移ったために、ローカル変数としてのスコープを抜けてもデストラクタに行かない、かつ、list側で使い終わるまで生存できるようになった、っぽい。
そのあたり、理解しきれてないので別途いろいろ試してみる。
別途確認したいモノ
unique_ptrは「1個だけ」しか存在できないもの。イメージは「参照カウンタが1より大きくできない」もの。
shared_ptrは「複数」存在できるもの。イメージは「参照カウンタが2以上にもなれる」もの
なので、今回の例の「Unique_ptrをshared_ptrにすると、moveをしなくてもlistに入れるはず。試したい。
→同じことができたとして、何が違うのか?も調べたい。
std::com_ptrも調べたい。
対策後コード (前段階)
上では対策としてスマートポインタを使ったが、スマートポインタの前段階として、
スタック上に作成されるMyClass mc
ではなく、ヒープ上に作成されるauto mc = new MyClass()
をvectorに入れることで、vectorの中のMyClassのインスタンスをdeleteするまで生存させることにしていた。
で、その時のコードは下記のような感じ。(差がある部分だけ)
std::vector<MyClass*> list; // ★ココ void CreateList() { auto mc = new MyClass();// ★ココ auto mc1= new MyClass();// ★ココ auto mc2 = new MyClass();// ★ココ list.push_back(mc);// ★ココ list.push_back(mc1);// ★ココ list.push_back(mc2);// ★ココ } int main() { CreateList(); wprintf(L"after func.\r\n"); for (int i = 0; i < list.size(); i++) { list[i]->func();// ★ココ delete list[i];// ★ココ } }
ただこれだと、deleteを自前で書く必要があり、C#に慣れている自分としては高確率で忘れてしまう。。。
→スマートポインタを使うことで、deleteをしなくて済むようにした。のが、上の対策後コードとなる。
ファクトリー
職場の方に言われて気づいたが、
このスマートポインタを使った方法(今回のCreateList()
関数)は、いわゆるファクトリーの用途として使えそう。
(引数で受けた値をもとに何かのクラスをnewして、newしたクラスを返す、C#でよくやるやつ)
参考
コピーコンストラクタはなぜ必要か?【C++】
コピーされる際、値がコピーされるのがよいが、ポインタがコピーされるのが問題、というのを知った。