Bug 134499

I forgot to mention that it takes *much longer* for the curves to appear than with the recent svn snapshot.

Created attachment 17879 [details]
Files to reprodice the bug

There are almost no differences between trunk and 1.3.0 so I suspect a bad build.

I know there aren't many differences, but there were locking issues until very recently and I suspect some cases have been missed. An unclean build is always a possibility, but I built 1.3.0 from a clean tarball, both on Linux and Solaris and in both cases I get the problem. Since it's a standard release build, I don't get much info in the backtrace but here is a hint:

#0  0x006857a2 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
#1  0x00617790 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/tls/libpthread.so.0
#2  0x004db7dd in KstWaitCondition::wait () from /ldk/0/soft/kde3.5/lib/libkst.so.1
#3  0x004bb023 in KstRWLock::readLock () from /ldk/0/soft/kde3.5/lib/libkst.so.1
#4  0x00427717 in KstDataObject::readLock () from /ldk/0/soft/kde3.5/lib/libkstmath.so.1
#5  0x00282dfd in KstObjectItem::update () from /ldk/0/soft/kde3.5/lib/libkstapp.so.1
#6  0x00281e8b in KstObjectItem::KstObjectItem () from /ldk/0/soft/kde3.5/lib/libkstapp.so.1
#7  0x00287b70 in KstDataManagerI::update () from /ldk/0/soft/kde3.5/lib/libkstapp.so.1
#8  0x00287890 in KstDataManagerI::show_I () from /ldk/0/soft/kde3.5/lib/libkstapp.so.1
#9  0x0028b5ec in KstDataManagerI::qt_invoke () from /ldk/0/soft/kde3.5/lib/libkstapp.so.1
#10 0x02d15a7c in QObject::activate_signal () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#11 0x02d158a4 in QObject::activate_signal () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#12 0x08f14880 in KAction::activated () from /ldk/0/soft/kde3.5/lib/libkdeui.so.4
#13 0x08f13af1 in KAction::slotActivated () from /ldk/0/soft/kde3.5/lib/libkdeui.so.4
#14 0x08f13ed2 in KAction::slotButtonClicked () from /ldk/0/soft/kde3.5/lib/libkdeui.so.4
#15 0x08f14b1c in KAction::qt_invoke () from /ldk/0/soft/kde3.5/lib/libkdeui.so.4
#16 0x02d15a7c in QObject::activate_signal () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#17 0x08fad0a6 in KToolBarButton::buttonClicked () from /ldk/0/soft/kde3.5/lib/libkdeui.so.4
#18 0x08fabbe3 in KToolBarButton::mouseReleaseEvent () from /ldk/0/soft/kde3.5/lib/libkdeui.so.4
#19 0x02d4ba17 in QWidget::event () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#20 0x08faca8a in KToolBarButton::event () from /ldk/0/soft/kde3.5/lib/libkdeui.so.4
#21 0x02cb99ff in QApplication::internalNotify () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#22 0x02cb90f4 in QApplication::notify () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#23 0x0074f2bc in KApplication::notify () from /ldk/0/soft/kde3.5/lib/libkdecore.so.4
#24 0x02c50220 in QETWidget::translateMouseEvent () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#25 0x02c4deac in QApplication::x11ProcessEvent () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#26 0x02c64c34 in QEventLoop::processEvents () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#27 0x02ccbbf8 in QEventLoop::enterLoop () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#28 0x02ccbaa8 in QEventLoop::exec () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#29 0x02cb9c51 in QApplication::exec () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#30 0x08053a11 in main ()

I'm getting the same hang with Kst from trunk...

Created attachment 17881 [details]
Unlock patch

This bug seems to be related to #133311 and commit #581128.

It seems that the scalars are locked even after the call to equation update. 
So, the patch just unlocks them after the update since there doesn't seem to be
an unlock anywhere in enodes.cpp.  I'm not sure if this is correct as perhaps
they are supposed to be unlocked somewhere else.

The patch seems to work here :-) Thanks, Adam ! 

Has this patch been included ? It seems to work fine, so if it is conceptually and syntactically correct, we should apply it and close this bug...

Right now this patch reverts the fix for bug #133311

Investigating...

SVN commit 593145 by treat:

* Do not allow equation parsing to proceed when
the equation references non-existent objects, whether
they be scalars, vectors or else.

BUG: 133311
BUG: 134499
BUG: 134996



 M  +18 -9     enodes.cpp  
 M  +5 -5      enodes.h  
 M  +10 -7     kstequation.cpp  


--- trunk/extragear/graphics/kst/src/libkstmath/enodes.cpp #593144:593145
@@ -93,7 +93,8 @@
 }
 
 
-void Node::collectObjects(KstVectorMap&, KstScalarMap&, KstStringMap&) {
+bool Node::collectObjects(KstVectorMap&, KstScalarMap&, KstStringMap&) {
+  return true;
 }
 
 
@@ -128,9 +129,11 @@
 }
 
 
-void BinaryNode::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
-  _left->collectObjects(v, s, t);
-  _right->collectObjects(v, s, t);
+bool BinaryNode::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+  bool ok = true;
+  ok = _left->collectObjects(v, s, t) ? ok : false;
+  ok = _right->collectObjects(v, s, t) ? ok : false;
+  return ok;
 }
 
 
@@ -600,8 +603,8 @@
 }
 
 
-void Function::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
-  _args->collectObjects(v, s, t);
+bool Function::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+  return _args->collectObjects(v, s, t);
 }
 
 
@@ -651,10 +654,12 @@
 }
 
 
-void ArgumentList::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+bool ArgumentList::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+  bool ok = true;
   for (Node *i = _args.first(); i; i = _args.next()) {
-    i->collectObjects(v, s, t);
+    ok = i->collectObjects(v, s, t) ? ok : false;
   }
+  return ok;
 }
 
 
@@ -853,7 +858,7 @@
 }
 
 
-void Data::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+bool Data::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
   if (_isEquation) {
     if (_equation) {
       _equation->collectObjects(v, s, t);
@@ -862,7 +867,11 @@
     v.insert(_tagName, _vector);
   } else if (_scalar && !s.contains(_tagName)) {
     s.insert(_tagName, _scalar);
+  } else {
+    KstDebug::self()->log(i18n("Equation has unknown object [%1].").arg(_tagName), KstDebug::Error);
+    return false;
   }
+  return true;
 }
 
 
--- trunk/extragear/graphics/kst/src/libkstmath/enodes.h #593144:593145
@@ -57,7 +57,7 @@
       virtual ~Node();
 
       virtual bool isConst() = 0; // can't be const
-      virtual void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      virtual bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       virtual bool takeVectors(const KstVectorMap& c);
       virtual double value(Context*) = 0;
       virtual void visit(NodeVisitor*);
@@ -75,7 +75,7 @@
       BinaryNode(Node *left, Node *right);
       virtual ~BinaryNode();
 
-      virtual void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      virtual bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       virtual bool takeVectors(const KstVectorMap& c);
       virtual void visit(NodeVisitor*);
       virtual KstObject::UpdateType update(int counter, Context *ctx);
@@ -102,7 +102,7 @@
       double value(Context*) { return 0.0; }
 
       bool isConst();
-      void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       bool takeVectors(const KstVectorMap& c);
       double at(int, Context*);
       Node *node(int idx);
@@ -124,7 +124,7 @@
       bool isPlugin() const;
       bool isConst();
       double value(Context*);
-      void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       bool takeVectors(const KstVectorMap& c);
       KstObject::UpdateType update(int counter, Context *ctx);
       QString text() const;
@@ -186,7 +186,7 @@
 
       bool isConst();
       double value(Context*);
-      void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       bool takeVectors(const KstVectorMap& c);
       KstObject::UpdateType update(int counter, Context *ctx);
       QString text() const;
--- trunk/extragear/graphics/kst/src/libkstmath/kstequation.cpp #593144:593145
@@ -268,9 +268,6 @@
   setDirty();
   _equation = in_fn;
   VectorsUsed.clear();
-  for (KstScalarMap::Iterator i = _inputScalars.begin(); i != _inputScalars.end(); ++i) {
-    (*i)->unlock();
-  }
   _inputScalars.clear();
   _ns = 2; // reset the updating
   delete _pe;
@@ -288,17 +285,23 @@
       ctx.xVector = *_xInVector;
       Equation::FoldVisitor vis(&ctx, &_pe);
       KstStringMap sm;
-      _pe->collectObjects(VectorsUsed, _inputScalars, sm);
-      for (KstScalarMap::Iterator i = _inputScalars.begin(); i != _inputScalars.end(); ++i) {
-        (*i)->readLock();
+      if (_pe->collectObjects(VectorsUsed, _inputScalars, sm)) {
+        _pe->update(-1, &ctx);
+      } else {
+        //we have bad objects...
+        KstDebug::self()->log(i18n("Equation [%1] references non-existent objects.").arg(_equation), KstDebug::Error);
+        _inputScalars.clear();
+        delete (Equation::Node*)ParsedEquation;
+        ParsedEquation = 0L;
+        Equation::mutex().unlock();
       }
-      _pe->update(-1, &ctx);
     } else {
       // Parse error
       KstDebug::self()->log(i18n("Equation [%1] failed to parse.  Errors follow.").arg(_equation), KstDebug::Warning);
       for (QStringList::ConstIterator i = Equation::errorStack.begin(); i != Equation::errorStack.end(); ++i) {
         KstDebug::self()->log(i18n("Parse Error: %1").arg(*i), KstDebug::Warning);
       }
+      _inputScalars.clear();
       delete (Equation::Node*)ParsedEquation;
       ParsedEquation = 0L;
       Equation::mutex().unlock();

> On Monday 09 October 2006 8:07 am, Brisset, Nicolas wrote:
> > Thanks for your work on this and the bugfixes, this problem 
> resulted 
> > in a lot of crashes recently and was becoming quite painful 
> :-) There 
> > is just one thing I'm wondering about: when I load existing .kst 
> > files, I get errors for scalars and vectors used in equations, even 
> > though they are in the file and eventually loaded correctly. I have 
> > the feeling that the loader delays some instantiations, which now 
> > generates a lot of meaningless (since in the end everything 
> is there) 
> > error messages. I don't know how the chain of events works, 
> so maybe 
> > I'm just wrong here, but the fact is I get lots of errors 
> (not warnings, errors) which I think I shouldn't be getting.
> 
> Hi Nicolas,
> 
> I'm not sure if this is the .kst file you are referring to, 
> but the one you included in 
> http://bugs.kde.org/show_bug.cgi?id=134499 referenced two 
> scalars which do not exist anywhere in the .kst file.  Can 
> you please make sure that the .kst files do in fact contain 
> the scalars and vectors?  Otherwise, feel free to send along 
> the .kst files or reproducable bugs and I'll have a look.

In the testcase_134499.kst file from the attachment, there are 2
equations referencing only available objects: all scalars are defined
above in the same file, and V-POSE and V-POSN are vectors produced by
plugins, also defined in the same file. If you look at the objects in
the datamanager you will see that they are all there. What's more, the
log output says:
"11/10/2006 14:21 Equation has unknown object [CST-PSI0_dg].
11/10/2006 14:21 Equation has unknown object [CST-dg2rd].
11/10/2006 14:21 Equation
[[CST-kt2ms]*(COS([CST-PSI0_dg]*[CST-dg2rd])*[V-POSE]+SIN([CST-PSI0_dg]*
[CST-dg2rd])*[V-POSN]+[CST-X0])] references non-existent objects."
and I have just checked, these scalars are defined.

So there is clearly something wrong here, like objects being reported as
unknown whereas they are defined. This is why I suspect delayed
instantiations to cause this...

this still happens for me when using scalars in an event monitor. should changes similar to those made for kstequation be made for events also?

duncan.

Duncan, could you provide exact steps to reproduce?  Thanks.

Sure,

1. Load some data... "kst -y 1 gyrodata.dat" works for me.
2. Then Data->New Event Monitor. Set expression to [1/First] or something.
3. Now click Ok. Everything will appear to be fine, until you try to enter
the Data Wizard or the Data Manager (possibly other things). This part is
probably why you haven't been able to reproduce yet- sorry, I didn't realize
that there was this requirement when I put in the original report!

Duncan.

On 30 Mar 2007 18:50:36 -0000, Adam Treat <treat@kde.org> wrote:
[bugs.kde.org quoted mail]
Sure,<br><br>1. Load some data... &quot;kst -y 1 gyrodata.dat&quot; works for me.<br>2. Then Data-&gt;New Event Monitor. Set expression to [1/First] or something.<br>3.
Now click Ok. Everything will appear to be fine, until you try to enter
the Data Wizard or the Data Manager (possibly other things). This part
is probably why you haven&#39;t been able to reproduce yet- sorry, I didn&#39;t
realize that there was this requirement when I put in the original
report!
<br><br>Duncan.<br><br><div><span class="gmail_quote">On 30 Mar 2007 18:50:36 -0000, <b class="gmail_sendername">Adam Treat</b> &lt;<a href="mailto:treat@kde.org">treat@kde.org</a>&gt; wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
------- You are receiving this mail because: -------<br>You are on the CC list for the bug, or are watching someone who is.<br><br><a href="http://bugs.kde.org/show_bug.cgi?id=134499">http://bugs.kde.org/show_bug.cgi?id=134499
</a><br><br><br><br><br>------- Additional Comments From treat kde org&nbsp;&nbsp;2007-03-30 20:50 -------<br>Duncan, could you provide exact steps to reproduce?&nbsp;&nbsp;Thanks.<br>_______________________________________________<br>Kst mailing list
<br><a href="mailto:Kst@kde.org">Kst@kde.org</a><br><a href="https://mail.kde.org/mailman/listinfo/kst">https://mail.kde.org/mailman/listinfo/kst</a><br></blockquote></div><br>

Thanks, I can confirm.

Created attachment 20162 [details]
Patch to fix deadlock

This fixes the deadlock, but updates are called continuously from
updatethread.cpp

Looks okay after a couple fixes:
1) Must remove the i18n() for now.  Just leave out that kstdebug line  
altogether.
2) I think there are some tabs or weird indenting in this patch?

--
George Staikos
KDE Developer				http://www.kde.org/
Staikos Computing Services Inc.		http://www.staikos.net/

SVN commit 650056 by treat:

* Fix for deadlock

BUG:134499


 M  +6 -2      ksteventmonitor_i.cpp  
 M  +6 -8      ksteventmonitorentry.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/ksteventmonitor_i.cpp #650055:650056
@@ -187,15 +187,19 @@
   }
 
   EventMonitorEntryPtr event = new EventMonitorEntry(tag_name);
+  event->writeLock();
   fillEvent(event);
 
   if (!event->isValid()) {
+    event->unlock();
     event = 0L;
 
     KMessageBox::sorry(this, i18n("There is a syntax error in the equation you entered."));
     return false;
   }
 
+  event->unlock();
+
   KST::dataObjectList.lock().writeLock();
   KST::dataObjectList.append(event.data());
   KST::dataObjectList.lock().unlock();
@@ -255,10 +259,10 @@
   } else if (_w->radioButtonLogError->isChecked()) {
     emPtr->setLevel(KstDebug::Error);
   }
-  
+
   emPtr->reparse();
   emPtr->unlock();
-  
+
   return true;
 }
 
--- trunk/extragear/graphics/kst/src/libkstapp/ksteventmonitorentry.cpp #650055:650056
@@ -118,6 +118,7 @@
 
 
 bool EventMonitorEntry::reparse() {
+  Q_ASSERT(myLockStatus() == KstRWLock::WRITELOCKED);
   _isValid = false;
   if (!_event.isEmpty()) {
     QMutexLocker ml(&Equation::mutex());
@@ -128,15 +129,12 @@
       Equation::Context ctx;
       Equation::FoldVisitor vis(&ctx, &_pExpression);
       KstStringMap stm;
-      _pExpression->collectObjects(_vectorsUsed, _inputScalars, stm);
-
-      for (KstScalarMap::ConstIterator i = _inputScalars.begin(); i != _inputScalars.end(); ++i) {
-        if ((*i)->myLockStatus() == KstRWLock::UNLOCKED) {
-          (*i)->readLock();
-        }
-      }
-
+      if (_pExpression->collectObjects(_vectorsUsed, _inputScalars, stm)) {
       _isValid = true;
+      } else {
+        //we have bad objects...
+        delete (Equation::Node*)ParsedEquation;
+      }
     } else {
       delete (Equation::Node*)ParsedEquation;
     }