Changeset 4e6ffe for src/UIElements

Timestamp:

Feb 14, 2016, 12:33:42 PM (9 years ago)

Author:

Frederik Heber <heber@…>

Branches:

Action_Thermostats, Add_AtomRandomPerturbation, Add_FitFragmentPartialChargesAction, Add_RotateAroundBondAction, Add_SelectAtomByNameAction, Added_ParseSaveFragmentResults, AddingActions_SaveParseParticleParameters, Adding_Graph_to_ChangeBondActions, Adding_MD_integration_tests, Adding_ParticleName_to_Atom, Adding_StructOpt_integration_tests, AtomFragments, Automaking_mpqc_open, AutomationFragmentation_failures, Candidate_v1.5.4, Candidate_v1.6.0, Candidate_v1.6.1, ChangeBugEmailaddress, ChangingTestPorts, ChemicalSpaceEvaluator, CombiningParticlePotentialParsing, Combining_Subpackages, Debian_Package_split, Debian_package_split_molecuildergui_only, Disabling_MemDebug, Docu_Python_wait, EmpiricalPotential_contain_HomologyGraph, EmpiricalPotential_contain_HomologyGraph_documentation, Enable_parallel_make_install, Enhance_userguide, Enhanced_StructuralOptimization, Enhanced_StructuralOptimization_continued, Example_ManyWaysToTranslateAtom, Exclude_Hydrogens_annealWithBondGraph, FitPartialCharges_GlobalError, Fix_BoundInBox_CenterInBox_MoleculeActions, Fix_ChargeSampling_PBC, Fix_ChronosMutex, Fix_FitPartialCharges, Fix_FitPotential_needs_atomicnumbers, Fix_ForceAnnealing, Fix_IndependentFragmentGrids, Fix_ParseParticles, Fix_ParseParticles_split_forward_backward_Actions, Fix_PopActions, Fix_QtFragmentList_sorted_selection, Fix_Restrictedkeyset_FragmentMolecule, Fix_StatusMsg, Fix_StepWorldTime_single_argument, Fix_Verbose_Codepatterns, Fix_fitting_potentials, Fixes, ForceAnnealing_goodresults, ForceAnnealing_oldresults, ForceAnnealing_tocheck, ForceAnnealing_with_BondGraph, ForceAnnealing_with_BondGraph_continued, ForceAnnealing_with_BondGraph_continued_betteresults, ForceAnnealing_with_BondGraph_contraction-expansion, FragmentAction_writes_AtomFragments, FragmentMolecule_checks_bonddegrees, GeometryObjects, Gui_Fixes, Gui_displays_atomic_force_velocity, ImplicitCharges, IndependentFragmentGrids, IndependentFragmentGrids_IndividualZeroInstances, IndependentFragmentGrids_IntegrationTest, IndependentFragmentGrids_Sole_NN_Calculation, JobMarket_RobustOnKillsSegFaults, JobMarket_StableWorkerPool, JobMarket_unresolvable_hostname_fix, MoreRobust_FragmentAutomation, ODR_violation_mpqc_open, PartialCharges_OrthogonalSummation, PdbParser_setsAtomName, PythonUI_with_named_parameters, QtGui_reactivate_TimeChanged_changes, Recreated_GuiChecks, Rewrite_FitPartialCharges, RotateToPrincipalAxisSystem_UndoRedo, SaturateAtoms_findBestMatching, SaturateAtoms_singleDegree, StoppableMakroAction, Subpackage_CodePatterns, Subpackage_JobMarket, Subpackage_LinearAlgebra, Subpackage_levmar, Subpackage_mpqc_open, Subpackage_vmg, Switchable_LogView, ThirdParty_MPQC_rebuilt_buildsystem, TrajectoryDependenant_MaxOrder, TremoloParser_IncreasedPrecision, TremoloParser_MultipleTimesteps, TremoloParser_setsAtomName, Ubuntu_1604_changes, stable

Children:

42d7dc

Parents:

04c3a3

git-author:

Frederik Heber <heber@…> (12/13/15 14:15:58)

git-committer:

Frederik Heber <heber@…> (02/14/16 12:33:42)

Message:

FIX: ObservedValue need to be removed with a bit of delay.

• ObservedValue has higher priority than e.g. QtObservedAtom. Hence, will also receive subjectKilled() before QtObservedAtom does. But right after all of these are received(), ObservedValueContainer will remove the instance which drops the shared ptr's count to zero (because Observer/Observables do not store shared_ptr but only ptrs!) and its dstor is called. Therefore, QtObservedAtom cannot access the ObservedValue any longer and this is against how the design was meant to be (ObservedValue lives at least as long as respective side in Qt needs access to its information).

Location:

src/UIElements/Qt4/InstanceBoard

Files:

• ObservedValuesContainer.hpp (modified) (3 diffs)
• ObservedValuesContainer_impl.hpp (modified) (7 diffs)
• QtObservedAtom.cpp (modified) (1 diff)
• QtObservedInstanceBoard.cpp (modified) (2 diffs)
• QtObservedInstanceBoard.hpp (modified) (1 diff)
• QtObservedMolecule.cpp (modified) (1 diff)

src/UIElements/Qt4/InstanceBoard/ObservedValuesContainer.hpp

@@ -79 +79 @@
   void countsubjectKilled(const id _id);
 
+  /** Prepares removeal a vector of observed values of an instance  identified by \a _id.
+   *
+   * \param _id identifier of instance
+   */
+  void removeObservedValues(const id _id);
+
   /** Erases a vector of observed values of an instance  identified by \a _id.
    *
    * \param _id identifier of instance
    */
-  void removeObservedValues(const id _id);
+  void eraseObservedValues(const id _id);
 
 private:
@@ -96 +102 @@
   //!> counts how many ObservedValues have already been subjectKilled()
   subjectKilledCount_t subjectKilledCount;
+
+  //!> typedef for the set with ids to be erase
+  typedef std::set<id> MarkedForErase_t;
+
+  //!> marks ids marked for erase (i.e. all subjectKilled() received)
+  MarkedForErase_t MarkedForErase;
 
   //!> name used in describing the instance type
@@ -122 +134 @@
    */
   bool checksubjectKilled(const id _id) const;
+
+  /** Internal function to check whether the vector of ObservedValue's
+   *  identified by \a _id has been marked for erase.
+   *
+   *  Marked for erase means that it has received all subjectKilled()
+   *  (the container not the values themselves).
+   *
+   * \param _id identifier of instance
+   * \return true - marked for erase, false - else
+   */
+  bool checkMarkedForErase(const id _id) const;
 
 private:

src/UIElements/Qt4/InstanceBoard/ObservedValuesContainer_impl.hpp

@@ -53 +53 @@
     const id _id)
 {
-  typename CountedObservedValues_t::iterator iter = ObservedValues.find(_id);
+  LOG(3, "DEBUG: ObservedValuesContainer got markObservedValuesAsConnected() for an observed value of "
+      << NameOfType << " " << _id);
+ typename CountedObservedValues_t::iterator iter = ObservedValues.find(_id);
   ASSERT (iter != ObservedValues.end(),
       "ObservedValuesContainer<T,id>::markObservedValuesAsConnected() - Observed value of "
@@ -72 +74 @@
     const id _id)
 {
+  LOG(3, "DEBUG: ObservedValuesContainer got markObservedValuesAsDisconnected() for an observed value of "
+      << NameOfType << " " << _id);
   typename CountedObservedValues_t::iterator iter = ObservedValues.find(_id);
   ASSERT (iter != ObservedValues.end(),
@@ -81 +85 @@
   --(iter->second.second);
 
-  if (checkRefCount(_id) && checksubjectKilled(_id))
+  if (checkRefCount(_id) && checksubjectKilled(_id) && checkMarkedForErase(_id))
     removeObservedValues(_id);
 }
@@ -109 +113 @@
   ++(iter->second);
 
-  if (checkRefCount(_id) && checksubjectKilled(_id))
+  if (checkRefCount(_id) && checksubjectKilled(_id) && checkMarkedForErase(_id))
     removeObservedValues(_id);
 }
@@ -121 +125 @@
   subjectKilledCount.erase(_id);
   ObservedValues.erase(_id);
+  MarkedForErase.erase(_id);
+}
+
+template <class T, typename id>
+void ObservedValuesContainer<T,id>::eraseObservedValues(const id _id)
+{
+#ifndef NDEBUG
+  std::pair< typename std::set<id>::iterator, bool > inserter =
+#endif
+  MarkedForErase.insert(_id);
+  ASSERT( inserter.second,
+      "ObservedValuesContainer<T,id>::eraseObservedValues() - received twice for "
+      +NameOfType+" "+toString(_id)+".");
+
+  if (checkRefCount(_id) && checksubjectKilled(_id) && checkMarkedForErase(_id))
+    removeObservedValues(_id);
+}
+
+template <class T, typename id>
+bool ObservedValuesContainer<T,id>::checkMarkedForErase(const id _id) const
+{
+  return MarkedForErase.count(_id);
 }
 
@@ -140 +166 @@
   const typename subjectKilledCount_t::iterator Solditer = subjectKilledCount.find(_oldid);
   const typename subjectKilledCount_t::iterator Snewiter = subjectKilledCount.find(_newid);
+  const typename MarkedForErase_t::iterator Eolditer = MarkedForErase.find(_oldid);
+  const typename MarkedForErase_t::iterator Enewiter = MarkedForErase.find(_newid);
   bool status = ((Colditer != ObservedValues.end()) && (Cnewiter == ObservedValues.end()));
   status &= ((Solditer != subjectKilledCount.end()) && (Snewiter == subjectKilledCount.end()));
+  status &= ((Eolditer != MarkedForErase.end()) && (Enewiter == MarkedForErase.end()));
   // change id here only if still present
   if (status) {
@@ -154 +183 @@
       subjectKilledCount.insert( std::make_pair(_newid, countvalue) );
     }
+    {
+      MarkedForErase.erase(Eolditer);
+      MarkedForErase.insert(_newid);
+    }
     return true;
   } else

src/UIElements/Qt4/InstanceBoard/QtObservedAtom.cpp

@@ -209 +209 @@
     owner = NULL;
 
-    if (!BoardIsGone)
+    emit atomRemoved();
+
+    if (!BoardIsGone) {
       board.markObservedAtomAsDisconnected(getAtomIndex());
-
-    emit atomRemoved();
+      board.markObservedAtomForErase(getAtomIndex());
+    }
   }
 }

src/UIElements/Qt4/InstanceBoard/QtObservedInstanceBoard.cpp

@@ -351 +351 @@
 }
 
+void QtObservedInstanceBoard::markObservedAtomForErase(const atomId_t _id)
+{
+  atomObservedValues.eraseObservedValues(_id);
+}
+
 void QtObservedInstanceBoard::markObservedMoleculeAsConnected(const moleculeId_t _id)
 {
@@ -360 +365 @@
   moleculeObservedValues.markObservedValuesAsDisconnected(_id);
 }
+
+void QtObservedInstanceBoard::markObservedMoleculeForErase(const moleculeId_t _id)
+{
+  moleculeObservedValues.eraseObservedValues(_id);
+}

src/UIElements/Qt4/InstanceBoard/QtObservedInstanceBoard.hpp

@@ -83 +83 @@
   void markObservedAtomAsConnected(const atomId_t _id);
   void markObservedAtomAsDisconnected(const atomId_t _id);
+  void markObservedAtomForErase(const atomId_t _id);
   void markObservedMoleculeAsConnected(const moleculeId_t _id);
   void markObservedMoleculeAsDisconnected(const moleculeId_t _id);
+  void markObservedMoleculeForErase(const moleculeId_t _id);
 
 signals:

src/UIElements/Qt4/InstanceBoard/QtObservedMolecule.cpp

@@ -172 +172 @@
     owner = NULL;
 
-    if (!BoardIsGone)
+    emit moleculeRemoved();
+
+    if (!BoardIsGone) {
       board.markObservedMoleculeAsDisconnected(getMolIndex());
-
-    emit moleculeRemoved();
+      board.markObservedMoleculeForErase(getMolIndex());
+    }
   }
 }