代入演算子を定義するときには「引数が自分自身」という罠に注意

「勉強がてら」書いたコードを実用的な目的に使おうとする人が出るとは思わなかったので注意書きを足しておいたshared_ptrを作るについて。奥さんや光成さんに指摘してもらった内容を忘れないようにメモしておきます。

この前のコードはこういう使い方をすると死にます。

int main(){
  {
    SPM x(new MyValue(43));
    cout << "x = x" << endl;
    x = x;
  }
}
-----
cstr with value43
x = x
obj 43 refcount--, now: 0
dstr43
obj -1073741821 refcount++, now: 1
obj -1073741821 refcount--, now: 0
dstr-1073741821
a.out(17254) malloc: *** error for object 0x100150: double free
*** set a breakpoint in malloc_error_break to debug
a.out(17254) malloc: *** error for object 0x100160: double free
*** set a breakpoint in malloc_error_break to debug

これは代入演算子が下のように定義されているので、x = x の時にまっさきにxをdeleteしてしまうからです。

  SharedPtr<T>& operator=(SharedPtr& rhs){
    if(_ptrwrap->dec() == 0){
      delete _ptrwrap;
    };
    rhs._ptrwrap->inc();
    _ptrwrap = rhs._ptrwrap;
    return *this;
  }

今回のケースに関してはincを先に呼ぶとか、最初に if(this == rhs) return *this; 足すこととかで回避できます。

ところでdeleteを先に呼んでしまうと、万が一 rhs._ptrwrap->inc() で例外が起きた場合にその後の _ptrwrap = rhs._ptrwrap; が呼ばれる前に外に出てしまうので、 delete済みの無効な「汚いポインタ」を持ったままの状態になってしまいます。そうするとその後の例外処理のコードの中とかで汚いポインタのせいで二次災害が起きたり、へんなエラーメッセージが出たり、エラーメッセージを出さずに死んだりしてしまいます。単に参照カウンタを増減しているだけなら例外が投げられることはないと思いますが、仮にnewしたりvectorを使ったりしてあればbad_alloc例外が投げられる可能性があります。

そこで、仮に例外が投げられても汚い状態にならないようにする方法を考えます。Effective C++に書かれているのをまねして、まず変更しようとしている値をコピーし、それを変更した後でswapでそれと取り替えることにします。

  SharedPtr<T>& operator=(SharedPtr& rhs){
    SharedPtr<T> copy(*this);
    SharedPtr<T> copy_rhs(rhs);
    
    copy._ptrwrap->dec();
    copy_rhs._ptrwrap->inc();
    this->swap(copy_rhs);
    rhs.swap(copy);
    return *this;
  }
  SharedPtr<T>& swap(SharedPtr<T>& rhs){
    std::swap(_ptrwrap, rhs._ptrwrap);
    return *this;
  }

コピーというコストを支払って、代わりに例外に対しての安全性を得ることができます。今回はここまでしなくても例外安全にすることはできたのですが、練習のためにこう書いてみました。こういう書き方をすると「代入の引数が自分自身ではないか？」と気にする必要がそもそも消えてなくなります。

参考文献
Effective C++ 第3版「operator=の実装では自己代入に備えよう」「コードを例外安全なものにしよう」、Exceptional C++。奥さん、光成さん、清水川さんには色々教えてもらいました。

差分だけ貼るつもりだったのですけど「コピーコンストラクタが必要」「copyを作ってswapした一時変数がスコープを抜けるタイミングで参照カウンタが下がって必要ならdeleteされないと行けないのでdeleteがSharedPtr側にあってはいけない」などなどでかなりかわったので貼り直します。

• • • • -

追記：
上ではもごもごしていたけども、下のようにスマートに書けるそうです。一歩ずつ追っていくとなるほど確かにこれでOKだ、ということはわかるんですけどなんかしっくりこない。

  SharedPtr<T>& operator=(const SharedPtr& rhs){
    SharedPtr<T> tmp(rhs);
    swap(tmp);
    return *this;
  }

実行結果
cstr with value43
x = x
obj 43 refcount++, now: 2
obj 43 refcount--, now: 1
end of assignment(43 must be alive. soon leave current scope)
obj 43 refcount--, now: 0
PtrWrapper dstr:43
dstr43
out of scope(43 must be deleted)

-----

cstr with value43
cstr with value44
x = y
obj 44 refcount++, now: 2
obj 43 refcount--, now: 0
PtrWrapper dstr:43
dstr43
after assign(43 must be deleted)
obj 44 refcount--, now: 1
out_of_y(44 must be alive)
obj 44 refcount--, now: 0
PtrWrapper dstr:44
dstr44
out_of_x(44 must be deleted)

-----

cstr with value45
45 should be accessible: 45
obj 45 refcount--, now: 0
PtrWrapper dstr:45
dstr45
45 should be deleted

#include <iostream>
using namespace std;

template<typename T>
class PtrWrapper {
public:
  PtrWrapper(T* ptr):_ptr(ptr),_count(1){}
  ~PtrWrapper(){
    cout << "PtrWrapper dstr:" << *_ptr << endl;
    delete _ptr;
  }

  void dec(){
    _count--;
    cout << "obj " << *_ptr << " refcount--, now: " << _count << endl;
    if(_count == 0){
      delete this;
    }
  }
  void inc(){
    _count++;
    cout << "obj " << *_ptr << " refcount++, now: " << _count << endl;
  }
  T* _ptr;
private:
  int _count;
};

template<typename T>
class SharedPtr {
public:
  SharedPtr(T* ptr):_ptrwrap(new PtrWrapper<T>(ptr)){
  }
  SharedPtr(const SharedPtr<T>& x):_ptrwrap(x._ptrwrap){
    _ptrwrap->inc();
  };
  ~SharedPtr(){
    _ptrwrap->dec();
  }
  T* operator->()const{
    return _ptrwrap->_ptr;
  }
  T operator*()const{
    return _ptrwrap->_ptr;
  }
  /*
  operator T*(){ // これは(T*)へのキャスト？
    return _ptrwrap._ptr;
  }
  */
  
  // operator=は自分への参照を返すこと
  SharedPtr<T>& operator=(const SharedPtr& rhs){
    SharedPtr<T> tmp(rhs);
    swap(tmp);
    return *this;
  }
  SharedPtr<T>& swap(SharedPtr<T>& rhs){
    std::swap(_ptrwrap, rhs._ptrwrap);
    return *this;
  }
  PtrWrapper<T> *_ptrwrap; // どうすえればprivateにできる？
};

class MyValue {
public:
  MyValue():value(42){
    cout << "cstr wit 42" << endl;
  }
  MyValue(int v):value(v){
    cout << "cstr with value" << value << endl;
  }
  ~MyValue(){
    cout << "dstr" << value << endl;
  }
  MyValue(const MyValue &v){
    cout << "copy cstr " << value << endl;
    value = v.value;
  }
  int value;
};

typedef SharedPtr<MyValue> SPM;
ostream& operator<<(std::ostream& os, const SPM& p){
  os << p->value;
  return os;
}
ostream& operator<<(std::ostream& os, const MyValue& v){
  os << v.value;
  return os;
}


SPM foo(){
  SPM x(new MyValue(45));
  return x;
}
int main(){
  {
    SPM x(new MyValue(43));
    cout << "x = x" << endl;
    x = x;
    cout << "end of assignment(" << x 
	 << " must be alive. soon leave current scope)" << endl;
  }
  cout << "out of scope(43 must be deleted)" << endl;

  cout << "\n-----\n" << endl;

  {
    SPM x(new MyValue(43));
    {
      SPM y(new MyValue(44));
      cout << "x = y" << endl;
      x = y;
      cout << "after assign(43 must be deleted)" << endl;
    }
    cout << "out_of_y(" << x << " must be alive)" << endl;
  }
  cout << "out_of_x(44 must be deleted)" << endl;

  cout << "\n-----\n" << endl;

  {
    SPM f(foo());
    cout << "45 should be accessible: " << f->value << endl;
  }
  cout << "45 should be deleted" << endl;
}