Version: 1.3.0_devel (using KDE 3.5.4, Kubuntu Package 4:3.5.4-0ubuntu2~dapper1 ) Compiler: Target: i486-linux-gnu OS: Linux (i686) release 2.6.15-26-686 Start kst Create an equation which refers to a non existent scalar (eg, [BLABLA]) The equation is evaluated, but is nonsense (maybe 0) because it refers to a non-existent scalar. Edit the equation and change the equation so it refers to something that actually does exist (eg, [C_PI]). kst: FATAL: Thread -1239136576 tried to unlock KstRWLock 0x818345c (unlocked) without holding the lock Either the equation should should not allow the bad equation to be created in the first place, or it should allow itself to get fixed without puking....
This crash happens if you increase the number of valid scalars in an equation. So, change an equation from x to x + [C_PI] and it will crash. It is not related to referring to an invalid scalar, except that fixing it increases the number of valid scalars, causing the crash.
Also it seems that there are some cases where Kst can get really messed up such as: kst --ye "2*[C_D]" foo.txt
This bug can also generate a hang: create an equation which uses [C_PI]. Edit the equation so that it no longer uses [C_PI]. Now create a new equation which uses [C_PI]: hang on [OK]. It looks like on apply equation edit, kst locks all scalars which were used by the equation, re-parses the equation, and then attempts to unlock all of the scalars which are now used. So -adding a scalar causes an attempt to unlock a not-locked scalar. -removing a scalar leaves it locked.
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();