読者です 読者をやめる 読者になる 読者になる

好き勝手に・げーあにん?

ファミコンと同い年の社会人ヌルオタの日記

あまりにも典型的なバグをばらまかないでください

C++ プログラム

ゲームコーディング・コンプリート 一流になるためのゲームプログラミング (Professional game programming)

ゲームコーディング・コンプリート 一流になるためのゲームプログラミング (Professional game programming)

『21.4 いろいろなバグ』の『21.4.1 メモリリークとヒープ破壊』のメモリリークが全然解決できていない上に、まだ正誤表も公開されていないようなので注意喚起。

4/23付けで正誤情報が公開されてた。


782ページから引用。

次に示すのは昔からよくあるメモリリークの例だ。このクラスはコンストラクタ内でメモリブロックを割り当てているが、仮想デストラクタを宣言していない。

class LeakyMemory : public SomeBaseClass
{
protected:
  int *leaked;

  LeakyMemory() { leaked = new int[128]; }
  ~LeakyMemory() { delete leaked; }
};

このコードは一見問題なさそうだが、メモリリークが発生する問題を内包している。このクラスがインスタンス化され、SomeBaseClassへのポインタによって参照されても、デストラクタが呼び出されることはない。

void main()
{
  LeakyMemory *ok = new LeakyMemory;
  SomeBaseClass *bad = new LeakyMemory;

  delete ok;
  delete bad; // ここでメモリリークが発生する
}

この問題を解決するには、LeakyMemoryの中でデストラクタをvirtualとして宣言すれば良い。

はい。C++をやってる人にはあまりにも基本中の基本過ぎて、今さら注意するまでもないんですが、最後の一文が見事に間違っていて、これでは解決できません。

そもそも、このコード、どう考えてもコンパイルも通りません。LeakyMemory のコンストラクタとデストラクタが protected では、インスタンス化ができずコンパイルエラーです。簡単なコードだからといってコンパイルを通る状態にしなかったことがバレバレですね*1

この問題を解決するには、LeakyMemoryの中でデストラクタをvirtualとして宣言すれば良い。

大事な間違いなので2回目の引用。LeakyMemory のデストラクタを virtual にしてもなんの解決にもなりません。デストラクタを virtual にするという解決方法を取るならば、SomeBaseClass のデストラクタに virtual を付けるべきです。~LeakyMemory には virtual 宣言があってもなくても(このコードから想像する範囲では)問題がありません。


あまりにも基本的すぎて書くのが億劫になってきましたが、この手のバグの根絶と、この手のバグを仕込む C++ プログラマの根絶を願って、本のコードから想像できる範囲で、正しく解決するコードを2通り置いておきます。

~SomeBaseClass を virtual にする

#include <cstdio>

class SomeBaseClass
{
public:
  virtual ~SomeBaseClass() {}
};

class LeakyMemory : public SomeBaseClass
{
private:
  int *leaked;
public:
  LeakyMemory() {/*printf("LeakyMemory()\n");*/ leaked = new int[128];}
  ~LeakyMemory() {delete[] leaked; /*printf("~LeakyMemory()\n");*/}
};

int main()
{
  LeakyMemory *ok = new LeakyMemory;
  SomeBaseClass *bad = new LeakyMemory;

  delete ok;
  delete bad;

  return 0;
}

SomeBaseClass を protected 継承にする

デストラクタが virtual になっていないクラスを継承した先で、デストラクタの処理が必要な場合は、アップキャストを封じるために、protected(もしくは private) 継承にする。

#include <cstdio>

class SomeBaseClass
{
};

class LeakyMemory : protected SomeBaseClass
{
private:
  int *leaked;
public:
  LeakyMemory() {/*printf("LeakyMemory()\n");*/ leaked = new int[128];}
  ~LeakyMemory() {delete[] leaked; /*printf("~LeakyMemory()\n");*/}
};

int main()
{
  LeakyMemory *ok = new LeakyMemory;
  // コンパイルエラー                                           
  //SomeBaseClass *bad = new LeakyMemory;                                       

  delete ok;
  //delete bad;                                                                 

  return 0;
}

関連

過去の自分のエントリー。これもC++のアップキャストでのバグ。C++プログラマのアップキャストでのバグは『ただの凡ミス』で済ませていいのか、根本的な理解が足りないのかで、指摘するときに悩む。

そんな事で悩むぐらいなら、静的解析ツールに指摘させればいいのか?

最後に

基本的基本的と連呼していて、私自信が間違った知識をばらまいてしまっていては本末転倒なので、少しでも間違いがあった際は、ご指摘をお願いします。

追記

コメントにて指摘を頂きました。配列newしてるのにdeleteが配列deleteになってませんでしたorz 情けないながらも言い訳をすると本のコードも配列deleteしてない……

*1:私もブログを書くときはよくやりますが(コラ