Bug 122927

Summary: Kst freezes when creating plot
Product: [Applications] kst Reporter: Andrew Walker <arwalker>
Component: generalAssignee: kst
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: 1.x   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Andrew Walker 2006-03-01 21:34:13 UTC
Version:           HEAD (using KDE KDE 3.5.0)
Installed from:    Compiled From Sources
OS:                Linux

STEPS TO REPRODUCE:
Start Kst
Configure the Plot update timer to the shortest time permissible
Create a single plot from the data wizard
Select Data...New Histogram...
Hit OK

RESULTS:
Kst freezes

EXPECTED RESULTS:
Kst produces the histogram
Comment 1 Andrew Walker 2006-03-01 21:36:01 UTC
The settings I used for the histogram were:

Place in new plot: checked
Regrid: checked
Columns: 1
Comment 2 George Staikos 2006-03-01 22:26:58 UTC
SVN commit 514906 by staikos:

Remove unnecessary and incorrect lock on the input vector to avoid a deadlock
BUG: 122927


 M  +3 -5      ksthsdialog_i.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/ksthsdialog_i.cpp #514905:514906
@@ -211,15 +211,14 @@
   KstHistogramPtr hs;
 
   KST::vectorList.lock().readLock();
-  KstVectorList::Iterator i = KST::vectorList.findTag(_w->_vector->selectedVector());
+  KstVectorPtr vp = *KST::vectorList.findTag(_w->_vector->selectedVector());
   KST::vectorList.lock().readUnlock();
-  if (i == KST::vectorList.end()) {
+  if (!vp) {
     kstdFatal() << "Bug in kst: the Vector field in plotDialog (Hs) refers to "
                 << " a non existant vector..." << endl;
   }
 
-  (*i)->readLock();
-  hs = new KstHistogram(tag_name, *i, new_min, new_max,
+  hs = new KstHistogram(tag_name, vp, new_min, new_max,
                         new_n_bins, new_norm_mode);
   hs->setRealTimeAutoBin(_w->_realTimeAutoBin->isChecked());
 
@@ -271,7 +270,6 @@
       }
     }
   }
-  (*i)->readUnlock();
 
   KST::dataObjectList.lock().writeLock();
   KST::dataObjectList.append(hs.data());
Comment 3 Andrew Walker 2006-03-02 01:22:55 UTC
The same problem occurs when creating a power spectrum
Comment 4 George Staikos 2006-03-03 21:06:45 UTC
SVN commit 515457 by staikos:

fix locking in PSDs, equations, and curves.  Also restore locking in histograms
since we might change the constructor one day.
BUG: 122927



 M  +43 -62    kstcurvedialog_i.cpp  
 M  +5 -3      ksteqdialog_i.cpp  
 M  +2 -0      ksthsdialog_i.cpp  
 M  +1 -1      kstpsddialog_i.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/kstcurvedialog_i.cpp #515456:515457
@@ -204,81 +204,77 @@
     return false;
   }
 
-  KstVectorList::Iterator VX, VY;
-  KstVectorList::Iterator EX, EY, EXMinus, EYMinus;
+  KstVectorPtr EXMinus, EYMinus;
 
   // find VX and VY
   KST::vectorList.lock().readLock();
-  VX = KST::vectorList.findTag(_w->_xVector->selectedVector());
-  VY = KST::vectorList.findTag(_w->_yVector->selectedVector());
-  EX = KST::vectorList.findTag(_w->_xError->selectedVector());
-  EY = KST::vectorList.findTag(_w->_yError->selectedVector());
+  KstVectorPtr VX = *KST::vectorList.findTag(_w->_xVector->selectedVector());
+  KstVectorPtr VY = *KST::vectorList.findTag(_w->_yVector->selectedVector());
+  KstVectorPtr EX = *KST::vectorList.findTag(_w->_xError->selectedVector());
+  KstVectorPtr EY = *KST::vectorList.findTag(_w->_yError->selectedVector());
   if (_w->_checkBoxXMinusSameAsPlus->isChecked()) {
     EXMinus = EX;
   } else {
-    EXMinus = KST::vectorList.findTag(_w->_xMinusError->selectedVector());
+    EXMinus = *KST::vectorList.findTag(_w->_xMinusError->selectedVector());
   }
   if (_w->_checkBoxYMinusSameAsPlus->isChecked()) {
     EYMinus = EY;
   } else {
-    EYMinus = KST::vectorList.findTag(_w->_yMinusError->selectedVector());
+    EYMinus = *KST::vectorList.findTag(_w->_yMinusError->selectedVector());
   }
-  if (VX == KST::vectorList.end() || VY == KST::vectorList.end()) {
+  KST::vectorList.lock().readUnlock();
+  if (!VX || !VY) {
     kstdFatal() << "Bug in kst: the XVector field in plotDialog refers to "
                 << "a non existent vector...." << endl;
   }
 
   // readlock the vectors, because when we update the curve, they get read
-  KstReadLocker rl1(*VX), rl2(*VY);
-  bool haveEx = EX != KST::vectorList.end();
-  bool haveEy = EY != KST::vectorList.end();
-  bool haveExMinus = EXMinus != KST::vectorList.end();
-  bool haveEyMinus = EYMinus != KST::vectorList.end();
-  if (haveEx) {
-    (*EX)->readLock();
-  }
-  if (haveEy) {
-    (*EY)->readLock();
-  }
-  if (haveExMinus) {
-    (*EXMinus)->readLock();
-  }
-  if (haveEyMinus) {
-    (*EYMinus)->readLock();
-  }
+  VX->readLock();
+  VY->readLock();
 
   QString tag_name = _tagName->text();
   if (tag_name == defaultTag) {
-    tag_name = KST::suggestCurveName((*VY)->tagName());
+    tag_name = KST::suggestCurveName(VY->tagName());
   }
 
   // verify that the curve name is unique
   if (KstData::self()->dataTagNameNotUnique(tag_name)) {
     _tagName->setFocus();
-    KST::vectorList.lock().readUnlock();
-    if (haveEx) {
-      (*EX)->readUnlock();
-    }
-    if (haveEy) {
-      (*EY)->readUnlock();
-    }
-    if (haveExMinus) {
-      (*EXMinus)->readUnlock();
-    }
-    if (haveEyMinus) {
-      (*EYMinus)->readUnlock();
-    }
     return false;
   }
 
+  if (EX) {
+    EX->readLock();
+  }
+  if (EY) {
+    EY->readLock();
+  }
+  if (EXMinus) {
+    EXMinus->readLock();
+  }
+  if (EYMinus) {
+    EYMinus->readLock();
+  }
+
   // create the curve
-  KstVCurvePtr curve = new KstVCurve(tag_name, *VX, *VY,
-                        haveEx ? *EX : KstVectorPtr(0L),
-                        haveEy ? *EY : KstVectorPtr(0L),
-                        haveExMinus ? *EXMinus : KstVectorPtr(0L),
-                        haveEyMinus ? *EYMinus : KstVectorPtr(0L),
-                        _w->_curveAppearance->color());
-  
+  KstVCurvePtr curve = new KstVCurve(tag_name, VX, VY, EX, EY, EXMinus, EYMinus, _w->_curveAppearance->color());
+
+  if (EX) {
+    EX->readUnlock();
+  }
+  if (EY) {
+    EY->readUnlock();
+  }
+  if (EXMinus) {
+    EXMinus->readUnlock();
+  }
+  if (EYMinus) {
+    EYMinus->readUnlock();
+  }
+
+  VY->readUnlock();
+  VX->readUnlock();
+
   QString legend_text = _legendText->text();
   if (legend_text == defaultTag) {
     curve->setLegendText(QString(""));
@@ -286,8 +282,6 @@
     curve->setLegendText(legend_text);
   }
   
-  KST::vectorList.lock().readUnlock();
-
   curve->setHasPoints(_w->_curveAppearance->showPoints());
   curve->setHasLines(_w->_curveAppearance->showLines());
   curve->setHasBars(_w->_curveAppearance->showBars());
@@ -331,19 +325,6 @@
     }
   }
 
-  if (haveEx) {
-    (*EX)->readUnlock();
-  }
-  if (haveEy) {
-    (*EY)->readUnlock();
-  }
-  if (haveExMinus) {
-    (*EXMinus)->readUnlock();
-  }
-  if (haveEyMinus) {
-    (*EYMinus)->readUnlock();
-  }
-
   KST::dataObjectList.lock().writeLock();
   KST::dataObjectList.append(curve.data());
   KST::dataObjectList.lock().writeUnlock();
--- trunk/extragear/graphics/kst/src/libkstapp/ksteqdialog_i.cpp #515456:515457
@@ -166,15 +166,17 @@
 
   KST::vectorList.lock().readLock();
   /* find *V */
-  KstVectorList::Iterator i = KST::vectorList.findTag(_w->_xVectors->selectedVector());
-  if (i == KST::vectorList.end()) {
+  KstVectorPtr vp = *KST::vectorList.findTag(_w->_xVectors->selectedVector());
+  if (!vp) {
     kstdFatal() << "Bug in kst: the Vector field in plotDialog (Eq) "
                 << "refers to a non-existent vector..." << endl;
   }
   KST::vectorList.lock().readUnlock();
 
   /** Create the equation here */
-  KstEquationPtr eq = new KstEquation(tag_name, _w->_equation->text(), *i, _w->_doInterpolation->isChecked());
+  vp->readLock();
+  KstEquationPtr eq = new KstEquation(tag_name, _w->_equation->text(), vp, _w->_doInterpolation->isChecked());
+  vp->readUnlock();
 
   if (!eq->isValid()) {
     eq = 0L;
--- trunk/extragear/graphics/kst/src/libkstapp/ksthsdialog_i.cpp #515456:515457
@@ -218,8 +218,10 @@
                 << " a non existant vector..." << endl;
   }
 
+  vp->readLock();
   hs = new KstHistogram(tag_name, vp, new_min, new_max,
                         new_n_bins, new_norm_mode);
+  vp->readUnlock();
   hs->setRealTimeAutoBin(_w->_realTimeAutoBin->isChecked());
 
   KstVCurvePtr vc = new KstVCurve(KST::suggestCurveName(tag_name, true), hs->vX(), hs->vY(), 0L, 0L, 0L, 0L, _w->_curveAppearance->color());
--- trunk/extragear/graphics/kst/src/libkstapp/kstpsddialog_i.cpp #515456:515457
@@ -179,6 +179,7 @@
                             _w->_kstFFTOptions->RateUnits->text(),
                             _w->_kstFFTOptions->ApodizeFxn->currentItem(),
                             _w->_kstFFTOptions->Sigma->value());
+    p->readUnlock();
 
     KstVCurvePtr vc = new KstVCurve(KST::suggestCurveName(tag_name,true), psd->vX(), psd->vY(), 0L, 0L, 0L, 0L, _w->_curveAppearance->color());
     vc->setHasPoints(_w->_curveAppearance->showPoints());
@@ -229,7 +230,6 @@
         }
       }
     }
-    p->readUnlock();
     KST::dataObjectList.lock().writeLock();
     KST::dataObjectList.append(psd.data());
     KST::dataObjectList.append(vc.data());