View Issue Details [ Jump to Notes ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0013120ParaView(No Category)public2012-04-24 02:572012-10-29 17:04
ReporterZhan 
Assigned ToSebastien Jourdain 
PriorityhighSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.14 
Target VersionFixed in Version3.98.0 
Summary0013120: Paraview crashed when deleting an added python macro from Macro Menu
DescriptionI have compiled ParaView 3.10.1 and 3.14.0 as 32bit on Windows 7 with MS VS 2008, and I found it would crash when I tried to delete a macro I added.

After debugging for some time, I found there's bug in the following codes:

for ( ; itr != this ->Internal->DeleteActionMap.constEnd(); ++itr )
    {
    if (itr.value() == action)
      {
          QString filename = itr.key();
          pqPythonMacroSupervisor::removeStoredMacro(filename);
          pqPythonMacroSupervisor::removeMacro(filename);
       }
    }
QMap's removing actions has different behavior on VS Compiler, and removing one element in this loop would cause dangling pointer problem.

I have changed the codes to the following(do not increment in for clauses) to fix this bug:
for ( ; itr != this->Internal->DeleteActionMap.constEnd();)
    {
    if (itr.value() == action)
      {
      QString filename = (itr++).key();
      pqPythonMacroSupervisor::removeStoredMacro(filename);
      pqPythonMacroSupervisor::removeMacro(filename);
      }
    else
    {
        ++itr;
    }
    }
TagsNo tags attached.
ProjectRelease
Topic Name13120-fix-delete-macro
Typecrash
Attached Files

 Relationships

  Notes
(0028744)
Lawrence (reporter)
2012-07-05 14:38

 I noticed pqPythonMacroSupervisor::removeMacro calls
this->Internal->DeleteActionMap.remove(fileName)
so is the iterator even still guaranteed to be valid?
(0028754)
Lawrence (reporter)
2012-07-06 11:59

 The following is a more robust fix.
(Started with 3.14.1 code, Fixes Dangling Iterator problem. Tested on Win7 64bit build of 3.14.1 - deleting macros no longer crashes Paraview)

void pqPythonMacroSupervisor::onDeleteMacroTriggered()
{
  QObject* action = this->sender();
  QMap<QString, QPointer<QAction> >::const_iterator itr = this->Internal->DeleteActionMap.constBegin();
  // (A more paranoid version would restart the iterator using DeleteActionMap.constBegin()).
  while( itr != this->Internal->DeleteActionMap.constEnd())
    {
    if (itr.value() == action)
      {
      QString filename = itr.key();
      ++itr; // Avoid dangling iterator; increment iterator before removeMacro modifies DeleteActionMap
      // "If you remove items from the map, iterators that point to the removed items will become dangling iterators."
      // - http://qt-project.org/doc/qt-5.0/qmap-iterator.html [^] , potentially causing a crash
      pqPythonMacroSupervisor::removeStoredMacro(filename);
      pqPythonMacroSupervisor::removeMacro(filename);
      }
    else
      {
      ++itr;
      }
    }
}
(0028898)
Utkarsh Ayachit (administrator)
2012-07-24 10:48

 merged into master, if applicable.
(0028935)
Alan Scott (manager)
2012-07-25 14:55

 Internal code fix. Closing. (Note that there really isn't enough information here to test.)

 Issue History
Date Modified Username Field Change
2012-04-24 02:57 Zhan New Issue
2012-07-05 14:38 Lawrence Note Added: 0028744
2012-07-06 11:59 Lawrence Note Added: 0028754
2012-07-19 09:10 Sebastien Jourdain Assigned To => Sebastiennn Jourdain
2012-07-19 09:10 Sebastien Jourdain Topic Name => 13120-fix-delete-macro
2012-07-19 09:10 Sebastien Jourdain Status backlog => gatekeeper review
2012-07-19 09:10 Sebastien Jourdain Resolution open => fixed
2012-07-24 10:47 Utkarsh Ayachit Fixed in Version => git-master
2012-07-24 10:48 Utkarsh Ayachit Status gatekeeper review => customer review
2012-07-24 10:48 Utkarsh Ayachit Note Added: 0028898
2012-07-25 14:55 Alan Scott Note Added: 0028935
2012-07-25 14:55 Alan Scott Status customer review => closed
2012-10-29 17:04 Utkarsh Ayachit Fixed in Version git-master => 3.98.0

Copyright © 2000 - 2018 MantisBT Team