vectorのイテレータがNULLになる

[タンタル]

こんにちは。
イテレータがよくわからない挙動を示しましたので、その質問です。

本来であればソースコードを示すべきとは思いますが、再現方法が自分でもわからないため、再現方法の取っ掛かりをつかめればと思い質問しました。
抽象的な話になると思いますが、よろしくお願い申し上げます。

コード:

void BulletManage::update(){
	for(vector<BulletBase*>::iterator it=this->bullet.begin();it!=this->bullet.end();){
		if((*it)->getValidate()==false){
			delete *it;
			it = this->bullet.erase(it);
			continue;
		}
		(*it)->update();
		++it;
	}
}

現在イテレータを上記のように使用しております。

bulletは、弾幕STGにおける弾クラスです。
しばらくは思った通りに弾が動きます。しかし、あるとき突然、"Vector Iterator not incrementable"というエラーが発生します。
止まった時の様子をデバッガで確かめたところ、itがNULLとなっていました。
なぜNULLになるのか...

とりあえず、[NULLならbreak]の条件文を入れて対応していますが、どうにも納得いきません。


このエラーは最初から出ていたわけではなく、これまでにも複数弾幕データを作ってきて、すべて問題なく動いてきました。
(今まで運が良かっただけ、ということはないと思いたいですが...)
新しく作った弾幕でのみ発生するエラーです。

この新しい弾幕データは何が違うのかが問題ですが、
新しく作ったものは、bulletがbulletを自己生成する、といえばいいのでしょうか。
bulletが、update()関数内でbulletをnewしてpush_backします。

それ問題なのかわかりませんが、これくらいしか僕の中では思い当たる節がありません...
実は、イテレータを使ってvectorにアクセスするときに、push_backしてはいけない、というルールがあるのでしょうか。

弾幕を作る際に、bulletの初期条件を与える必要があるのですが、このときの加速度を0.01から0.012にしたり、同時に発生する数を15から10にしたらエラーが発生しなくなった、
などが起こったため、別のところに原因がある気がしますが...
（それとも運よくエラーが発生しなかっただけ？）


抽象的で情報不足な質問で申し訳ありません。
上記の使い方に問題がないようでしたら、問題ないと教えていただけるだけでも幸いです。
よろしくお願いします。

[usao]

>bulletが、update()関数内でbulletをnewしてpush_backします。

this->bullet と同じvectorにですか？
だとしたらこれが原因でしょう．
push_back()したらその直前までの要素を指す参照やiteratorの類は無効になり得ます．

オフトピック

（実際のvectorの実装がどうなのかは別として）
例えば５個の要素を持っているvectorに新たな要素をpush_backするとき，
vectorがその6個目の要素を保持できるメモリをあらかじめ持っていなかったら…？
的なことを想像すれば何となくわかるかと．

[みけCAT]

この新しい弾幕データは何が違うのかが問題ですが、
新しく作ったものは、bulletがbulletを自己生成する、といえばいいのでしょうか。
bulletが、update()関数内でbulletをnewしてpush_backします。

それ問題なのかわかりませんが、これくらいしか僕の中では思い当たる節がありません...
実は、イテレータを使ってvectorにアクセスするときに、push_backしてはいけない、というルールがあるのでしょうか。

これに近いようです。
http://www.open-std.org/jtc1/sc22/wg21/ ... /n3337.pdf

Remarks: Reallocation invalidates all the references, pointers, and iterators referring to the elements
in the sequence. It is guaranteed that no reallocation takes place during insertions that happen after
a call to reserve() until the time when an insertion would make the size of the vector greater than
the value of capacity().

void push_back(const T& x);
void push_back(T&& x);
1 Remarks: Causes reallocation if the new size is greater than the old capacity. If no reallocation happens,
all the iterators and references before the insertion point remain valid. If an exception is thrown other
than by the copy constructor, move constructor, assignment operator, or move assignment operator
of T or by any InputIterator operation there are no effects. If an exception is thrown by the move
constructor of a non-CopyInsertable T, the effects are unspecified.

push_backする際に要素数がcapacity()を超えてしまうとreallocationが発生し、イテレータが無効になるようです。

[みけCAT]

this->bulletの要素にランダムアクセスする必要がなければ、追加や削除がその操作の対象以外をinvalidateしないstd::listを使うといいかもしれません。

void push_front(const T& x);
void push_front(T&& x);
void push_back(const T& x);
void push_back(T&& x);
1 Remarks: Does not affect the validity of iterators and references. If an exception is thrown there are
no effects.
(中略)
iterator erase(const_iterator position);
iterator erase(const_iterator first, const_iterator last);
void pop_front();
void pop_back();
void clear() noexcept;
3 Effects: Invalidates only the iterators and references to the erased elements.

[タンタル]

みけCATさん
ありがとうございます。

同じ条件でもエラーが出たりでなかったりですし、
例外処理でも逃げきれなくて困っておりました。
なるほど、vectorは反復中に要素追加してはいけないのですね。大変助かりました。ありがとうございます。

ただせっかくのご意見なのですが、ランダムアクセスはかなり頻繁に行うので、vectorは手放せないです...
申し訳ありません...

うーん...
皆さんは繰り返し中に要素追加とかしないのでしょうか。

とりあえずですが、
・bulletとは別にリストbullet_tempを準備しておく。
・update()関数内からpush_backするときは、bullet_tempにする。
・bulletのループが一度終了しきってから、bullet_tempの中身をbulletに移動。
という方法を考えましたので、こちらを試してみたいと思います。
（間違っていれば指摘していただけると幸いです。）

始めからキャパシティを多くとっておくことも考えましたが、その場しのぎで本質的な解決にはならなさそうですね。
弾がどこまで増えるかは未知数ですし、メモリの無駄使いにもつながりそうです。

知りたかった原因について判明しましたので、解決といたします。ありがとうございました。
他にも方法がありましたら、ご教示いただけると幸いです。

[タンタル]

usaoさん
スルーしてしまいました。申し訳ございません。
ご回答いただきありがとうございました。

[YuO]

イテレータの削除だけであれば，std::remove_if使って最後にまとめてstd::vectorから削除，という方法があるのですが，ループ中に追加もあるのですよね。
そうであれば，std::vectorなのですから，添字でアクセスするのが簡単かと。
追加をpush_back，または最後尾に行う限り，該当要素の添字は変化しませんから。

[タンタル]

YuOさん
ありがとうございます。

添え字でアクセスするというのは、
for(int i=0;i<this->bullet.size();i++)
ということでしょうか？

コード:

for (int i = 0; i < this->bullet.size();){
	if (this->bullet[i]->getValidate() == false){
		delete this->bullet[i];
		this->bullet.erase(this->bullet.begin()+i);
		continue;
	}
	this->bullet[i]->update();
	++i;
}

にしてみました。
今のところエラーで落ちる様子はありません。
こちらですと余計な変数を増やす必要がなくなりますし、「こういうときだけはtempを使う」といった場合わけがなくなるので、
とてもうれしいですね。

間違っていましたら、ご指摘いただけると幸いです。

[usao]

どうでも良い点かと思いますが

>・bulletとは別にリストbullet_tempを準備しておく。
>・update()関数内からpush_backするときは、bullet_tempにする。
>・bulletのループが一度終了しきってから、bullet_tempの中身をbulletに移動。

と，添え字でアクセスする方法とでは，
今回の処理で増えた要素のupdate()が今回処理されるかどうか　という点で異なりそうです．

[タンタル]

usaoさん
ありがとうございます。

添え字を使う方法では、自動的に末尾に追加され、size()も増えていくので、新しくできたものも今のループでupdateされますが、
tempを使う方法では、updateし終わってからループに追加されるので、次のループでないとupdateされないですね。

その点は私も少し気にしたのですが、一フレームの厳密さを求めていなかったことと、
そもそもここ以外の部分でフレーム管理はぐちゃぐちゃになっていましたので、今回は省略しました。
(count変数を、update()の始めでインクリメントするか、終わりでするか、などを適当に管理してるので、時々困ったことになります(笑))
tempを使う方法では、今回の場合では本線に加える前にupdateすることで、フレームのずれは解消できそうですね。

真っ先に回答していただいたにも関わらず、お礼が遅くなってしまい、大変申し訳ありませんでした。