fix release mouse crash, add missed disconnects, remove unused callback

(bzr r6430)

3 files changed, 14 insertions, 13 deletions

diff --git a/src/knot-holder-entity.cpp b/src/knot-holder-entity.cpp
index e2ae5f10e..20ab01143 100644
--- a/src/knot-holder-entity.cpp
+++ b/src/knot-holder-entity.cpp
@@ -55,6 +55,10 @@ KnotHolderEntity::create(SPDesktop *desktop, SPItem *item, KnotHolder *parent, c
 
 KnotHolderEntity::~KnotHolderEntity()
 {
+    _moved_connection.disconnect();
+    _click_connection.disconnect();
+    _ungrabbed_connection.disconnect();
+
     /* unref should call destroy */
     if (knot) {
         g_object_unref(knot);
diff --git a/src/knot-holder-entity.h b/src/knot-holder-entity.h
index e4564cd01..a29b71f4d 100644
--- a/src/knot-holder-entity.h
+++ b/src/knot-holder-entity.h
@@ -50,7 +50,6 @@ public:
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state) = 0;
     virtual NR::Point knot_get() = 0;
     virtual void knot_click(guint /*state*/) {}
-    virtual void onKnotUngrabbed() {}
 
     void update_knot();
 
diff --git a/src/knotholder.cpp b/src/knotholder.cpp
index f0495a244..f2bbbfbc0 100644
--- a/src/knotholder.cpp
+++ b/src/knotholder.cpp
@@ -161,28 +161,26 @@ KnotHolder::knot_ungrabbed_handler(SPKnot *knot)
         this->released(this->item);
     } else {
         SPObject *object = (SPObject *) this->item;
+
+        // Caution: this call involves a screen update, which may process events, and as a
+        // result the knotholder may be destructed. So, after the updateRepr, we cannot use any
+        // fields of this knotholder (such as this->item), but only values we have saved beforehand
+        // (such as object).
         object->updateRepr();
 
         /* do cleanup tasks (e.g., for LPE items write the parameter values
          * that were changed by dragging the handle to SVG)
          */
-        if (SP_IS_LPE_ITEM(item)) {
-            // This writes all parameters to SVG. Is this sufficiently efficient or should we only write
-            // the ones that were changed (e.g., via the individual handles' onKnotUngrabbed() method?            
-            Inkscape::LivePathEffect::Effect *lpe = sp_lpe_item_get_current_lpe(SP_LPE_ITEM(item));
+        if (SP_IS_LPE_ITEM(object)) {
+            // This writes all parameters to SVG. Is this sufficiently efficient or should we only
+            // write the ones that were changed?
+
+            Inkscape::LivePathEffect::Effect *lpe = sp_lpe_item_get_current_lpe(SP_LPE_ITEM(object));
             if (lpe) {
                 LivePathEffectObject *lpeobj = lpe->getLPEObj();
                 SP_OBJECT(lpeobj)->updateRepr();
             }
         }
-        // this was once used to write individual parameter values to SVG but this is now done globally above;
-        // we leave the calls to onKnotUngrabbed, anyway, in case any other cleanup tasks need to be done
-        for(std::list<KnotHolderEntity *>::iterator i = this->entity.begin(); i != this->entity.end(); ++i) {
-            KnotHolderEntity *e = *i;
-            if (e->knot == knot) {
-                e->onKnotUngrabbed(); // for most KnotHolderEntitys this does nothing
-            }
-        }
 
         unsigned int object_verb = SP_VERB_NONE;