Version: 1.3.0 (using KDE 3.4.0, compiled sources) Compiler: gcc version 3.4.3 OS: SunOS (sun4u) release 5.8 There is a problem, apparently linked with locks once again, that results in a reproducible crash. The console output says: "kst: FATAL: Thread 1 tried to unlock KstRWLock 5f6ffc (unlocked) without holding the lock KCrash: crashing... crashRecursionCounter = 2" To reproduce: 1) start kst, create V10 from 0 to 180, 10 values 2) create V100 from 0 to 180, 100 values 3) create an equation with the formula "sin([V10]*[C_D2R])" and V10 as X vector, click OK: the curve is created 4) edit the equation, change X vector to V100, click OK: crash! I have colleagues working with kst and lots of equations. They have an impressive talent in finding bugs :-) Even though it feels bad when they yell out "oh no, it crashed again!", they are always impressed when I tell them the fix is installed the next day. I hope it will be the case again ;-) ?
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();