ofTheo / ofxKinect

legacy openFrameworks wrapper for the xbox kinect (OF pre-0.8.0+ only) - ofxKinect is now included and is being maintained in OF releases
MIT License
540 stars 105 forks source link

Crash on shutdown caused by invalid iterators in the ofxKinectContext::closeAll function #111

Closed mrjman closed 11 years ago

mrjman commented 11 years ago

The error message in debug mode is something to the effect of: expression map/set iterator not incrementable.

The problem is caused by ofxKinectContext::closeAll looping over the ofxKinect map and calling ofxKinect::close() which triggers ofxKinectContext::close which finally erases the iterator from the map. The map.erase() is the heart of the problem because this invalidates the map iterators causing ofxKinectContext::closeAll to access an out of range iterator.

One solution to this problem which does not involve refactoring how closeAll and close work is to simply make a temporary copy of the map in closeAll and then loop over this temporary map calling close on each device. This avoids the invalid iterators and should be safe because only ofxKinect::close is being called. Also since this is only done on shutdown and the kinect map will never be large I don't see this solution as a large performance impact.

The closeAll snippet for the solution described above:

void ofxKinectContext::closeAll() {
    std::map<int, ofxKinect*> tempMap(kinects);
    std::map<int,ofxKinect*>::iterator iter;
    for(iter = tempMap.begin(); iter != tempMap.end(); ++iter) {
        iter->second->close();
    }
}
danomatika commented 11 years ago

Yeah I sometimes forget about invalidating the iterators. If you make a PR, I'll accept it.

danomatika commented 11 years ago

Actually, never mind. I'll just take your snippet above.