Skip to content

Commit

Permalink
Fixed|libdeng2: Crash when recreating canvas
Browse files Browse the repository at this point in the history
When the audiences of the old canvas were copied to the new one,
the C++ default assignment operator was used. This meant a pointer
to the old audiences was copied, causing a crash when continuing
iteration through a deleted audience.

Todo for later: This could use further improvement, perhaps by
utilizing a QSet in Observers<>. If the observers set is destroyed
in the middle of iterating through a large number of observers,
there will likely still be a crash because the 'next' iterator of
the loop will become invalid (and not just point to the end of the set
as in this instance when there is a single observer).

Observers<>::Loop could also observe the set itself, although in
that case it would be necessary to derive a custom set class that
provides the necessary notifications to Loop.
  • Loading branch information
skyjake committed Apr 11, 2013
1 parent 557bae3 commit a2af307
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
31 changes: 29 additions & 2 deletions doomsday/libdeng2/include/de/data/observers.h
Expand Up @@ -113,9 +113,16 @@ namespace de
{
/**
* Template for observer sets. The template type should be an interface
* implemented by all the observers.
* implemented by all the observers. @ingroup data
*
* @ingroup data
* @todo This could use further improvement, perhaps by utilizing a QSet in
* Observers<>. If the observers set is destroyed in the middle of
* iterating through a large number of observers, there will likely still
* be a crash because the 'next' iterator of the loop will become invalid
* (and not just point to the end of the set as in this instance when there
* is a single observer). Observers<>::Loop could also observe the set
* itself, although in that case it would be necessary to derive a custom
* set class that provides the necessary notifications to Loop.
*/
template <typename Type>
class Observers
Expand Down Expand Up @@ -166,6 +173,10 @@ namespace de
public:
Observers() : _members(0) {}

Observers(Observers<Type> const &other) : _members(0) {
*this = other;
}

virtual ~Observers() {
clear();
}
Expand All @@ -175,6 +186,22 @@ namespace de
_members = 0;
}

Observers<Type> &operator = (Observers<Type> const &other) {
// If _members already exists, the instance is retained
// in case someone is using it.
if(other._members) {
checkExists();
_members->clear();
DENG2_FOR_EACH_CONST(typename Members, i, *other._members) {
add(*i);
}
}
else {
if(_members) _members->clear();
}
return *this;
}

/// Add an observer into the set. The set does not receive
/// ownership of the observer instance.
void add(Type *observer) {
Expand Down
2 changes: 1 addition & 1 deletion doomsday/libgui/src/canvas.cpp
Expand Up @@ -278,7 +278,7 @@ void Canvas::notifyReady()

DENG2_FOR_AUDIENCE(GLReady, i) i->canvasGLReady(*this);

// The Canvas instance might have been destroyed now.
// This Canvas instance might have been destroyed now.
}

void Canvas::paintGL()
Expand Down

0 comments on commit a2af307

Please sign in to comment.