Threads

[Querdenker]

Hi Alle,
nachdem ich meine Threads nun erfolgreich starten kann
ein Problemchen, nämlich race conditions zu vermeiden.

Ich habe hier eine Threadclasse die viele Daten über Eigenschaften entgegen nimmt und dann über start() und run() eine xml Datei bedient. Das klappt, das klappt auch mehrfach hintereinander, allerdings nicht immer: Die Threadklasse ist also nicht threadsafe.
Mit QMutex und QWaitCondition komme ich nicht klar, weil QWaitCondition immer sperrt. QThreadStorage (TLS) wäre mir zu umständlich.
Hat jemand eine Idee, wie man das bewerkstelligen könnte?

hier ist die Headerfile der Klasse, es folgt in der Implementierung nur noch die run() Methode mit der Speicherung der Daten in eine XML-Datei:

Code: Alles auswählen

#ifndef m_SAVER_H
#define m_SAVER_H
#include <QThread>
#include "sax2.h"

class QString;
class QColor;
class SAX2;

class Saver : public QThread{

private:
   QString    TabellenName;
   SAX2       *xmlfile;
   int        Mode;
   int        rowID;
   int        colID;
   QString    sAttr;
   int        nPos;
   QString    sFont;
   int        nSize;
   bool       bBold;
   bool       bItalic;
   bool       bUnderl;
   bool       bUnters;
   int        nWidth;
   int        nHeight;
   QColor     backcolor;
   QColor     forecolor;
   void       run();
   
public:
   void setXMLFile(SAX2 *pSax)          { xmlfile = pSax; };
   void setMode(int nMode)              { Mode = nMode; };
   void setTabName(QString sTab)        { TabellenName = sTab; };
   void setRowID(int row)               { rowID = row; };
   void setColID(int col)               { colID = col; };
   void setAttr(QString sAt)            { sAttr = sAt; };
   void setPos(int np)                  { nPos = np; };
   void setFont(QString sf)             { sFont = sf; };
   void setSize(int s)                  { nSize = s; };
   void setBold(bool b)                 { bBold = b; };
   void setItalic(bool i)               { bItalic = i; };
   void setUnderLine(bool u)            { bUnderl = u; };
   void setWidth(int w)                 { nWidth = w; };
   void setHeight(int h)                { nHeight = h; };
   void setBackcolor(QColor bc)         { backcolor = bc; };
   void setForecolor(QColor fc)         { forecolor = fc; };
};
#endif

Und diese Action hier feuert sie:

Code: Alles auswählen

for(i=0;i<qr.count();i++){
	   qr[i]->setBackgroundColor(r);
	   if(xmlfile != NULL){
	      savers[i] = new Saver();
	      savers[i]->setTabName(tb);
	      savers[i]->setMode(2);
	      savers[i]->setRowID(qt->item(inx[i].row(), pos)->text().toInt());
	      savers[i]->setPos(inx[i].row());
	      savers[i]->setAttr("BackColor");
	      savers[i]->setBackcolor(r);
	      savers[i]->setColID(inx[i].column());
	      savers[i]->setHeight(qt->rowHeight(inx[i].row()));
	      savers[i]->setWidth(qt->columnWidth(inx[i].column()));
	      savers[i]->setXMLFile(xmlfile);
	      //Saver-Thread start ...
	      savers[i]->start(QThread::HighPriority);
	   }
   }

[upsala]

Definiere: 'das klappt nicht'.

Rufst du das selbe Object mehrmals auf, oder rufst du verschiedene Objekte mehrmals auf?

[Querdenker]

Hmm,
ich fange am besten von vorne an

Meine Absicht ist es die Einstellungen die ein User an einer oder mehrerer QTableWidget vornimmt persistent zu machen. D.h. wenn der User Rows oder Columns ändert, oder Farben und Schrift ändert, will ich das in einer XML-Datei persistent machen, damit der Zustand einer Tabelle jederzeit reproduzierbar ist. Die Tücke des Objects ist dabei aber, dass der User mehrere Zellen markiert haben könnte - oder manche Aktionen eben mehrere Zellen betreffen. In diesem Fall wird die Angelegenheit faktisch im Hintergrund via Threads bearbeitet, damit der User auf seiner Tabelle weiterarbeiten kann - ohne vom dem Speicherprozess etwas zu merken, was durchaus Zeitintensiv sein könnte.

Genau dann wenn mehrere Zellen hintereinander gespeichert werden laufe ich in eine race condition hinein, weil mein Proggie bislang nicht threadsafe beproggt wurde - das soll es noch werden .

Momentan baue ich meine Klassen threadsafe um. Damit die Threads wirklich auf wohl definierten Zuständen laufen, verwende ich nun doch QThreadStorage (TLS) und sichere soweit es geht mit Mutex.

So habe ich nun bei der Klasse begonnen, die am Ende der Kette liegt: Die eben das XML-File versorgt. Sie ist kein Thread, aber wird hauptsächlich von Threads gefahren:

Code: Alles auswählen

#include "xmldom.h"
#include <QApplication>
#include <QString>
#include <QMutex>
#include <QStringList>
#include <QTextStream>
#include <QColor>
#include <QFile>
#include <QDomElement>

void XmlDom::saveDOM(){
   mx.lock();
   QFile fil(ActXmlFile);
   if(!fil.open(QIODevice::WriteOnly)) {
       mx.unlock();
	    return;
	}
   QTextStream out(&fil);
   save(out, 1);
   fil.close();
   mx.unlock();
};

bool XmlDom::DelTable(QString sTabName){
   QDomElement t = findElement(sTabName,"", "", 0);
   mx.lock();
   if(!t.isNull()){
      mainE.removeChild(t);
      mx.unlock();
      return true;
   }
   mx.unlock();
   return false;
};

bool XmlDom::DelRow(QString sTabName, int nRow){
   QDomElement t= findElement(sTabName, "", "", 0);
   if(!t.isNull()){
      QDomElement r = findElement(sTabName,GetRowName(nRow), "", 1);
      mx.lock();
      if(!r.isNull()){
         t.removeChild(r);
         mx.unlock();
         return true;
      }
      mx.unlock();
   }
   return false;
};

bool XmlDom::DelCol(QString sTabName, int nRow, int nCol){
   QDomElement r = findElement(sTabName, GetRowName(nRow), "", 1);
   if(!r.isNull()){
      QDomElement c = findElement(sTabName, GetRowName(nRow), GetColName(nCol), 2);
      mx.lock();
      if(!c.isNull()){
         r.removeChild(c);
         mx.unlock();
         return true;
      }
      mx.unlock();
   }
   return false;
};

bool XmlDom::RenameTable(QString sOldName, QString sNewName){
   QDomElement t = findElement(sOldName, "", "", 0);
   mx.lock();
   if(!t.isNull()){
      t.setTagName(sNewName);
      mx.unlock();
      return true;
   }
   mx.unlock();
   return false;
};

bool XmlDom::isTableExist(QString sTab){
   mx.lock();
   bool b = false;
   QDomElement t = mainE.firstChildElement(sTab);
   if(!t.isNull())b = true;
   mx.unlock();
   return b;
};


bool XmlDom::isRowExist(QString sTab, int nRow){
   mx.lock();
   bool b = false;
   QString tn = GetRowName(nRow);
   QDomElement t = mainE.firstChildElement(sTab);
   if(!t.isNull()){
      QDomElement r = t.firstChildElement(tn);
      if(!r.isNull()) b = true;
   }
   mx.unlock();
   return b;
};

bool XmlDom::isColExist(QString sTab, int nRow, int nCol){
   mx.lock();
   bool b = false;
   QString tn = GetRowName(nRow);
   QString cn = GetColName(nCol);
   QDomElement t = mainE.firstChildElement(sTab);
   if(!t.isNull()){
      QDomElement r = t.firstChildElement(tn);
      if(!r.isNull()){
         QDomElement c = r.firstChildElement(cn);
         if(!c.isNull()) b = true;
      }
   }
   mx.unlock();
   return b;
};

void XmlDom::insertIntoCache(QString id, QString s){
   if(!cache.hasLocalData()) cache.setLocalData(new QMap<QString, QString>);
   cache.localData()->insert(id, s);
};

void XmlDom::removeFromCache(QString id){
   if(cache.hasLocalData()) cache.localData()->remove(id);
};

QString XmlDom::readFromCache(QString id){
   if(cache.hasLocalData())
      return cache.localData()->value(id);
         else
            return "";
};

void XmlDom::setRowID(int nRowID){
   mx.lock();
   QString t;
   t.setNum(nRowID);
   insertIntoCache("RowID", t);
   mx.unlock();
};

void XmlDom::setRowPos(int nPos){
   mx.lock();
   QString t;
   t.setNum(nPos);
   insertIntoCache("RowPos",t);
   mx.unlock();
};

void XmlDom::setRowHeight(int nHeight){
   mx.lock();
   QString t;
   t.setNum(nHeight);
   insertIntoCache("RowHeight", t);
   mx.unlock();
};

void XmlDom::setAlign(int nAlign){
   mx.lock();
   QString t;
   t.setNum(nAlign);
   insertIntoCache("Align", t);
   mx.unlock();
};

void XmlDom::setWidth(int n){
   mx.lock();
   QString t;
   t.setNum(n);
   insertIntoCache("Width",t);
   mx.unlock();
};

void XmlDom::setSize(int n){
   mx.lock();
   QString t;
   t.setNum(n);
   insertIntoCache("Size", t);
   mx.unlock();
};

void XmlDom::setFont(QString s){
   mx.lock();
   insertIntoCache("Size", s);
   mx.unlock();
};

void XmlDom::setBold(bool b){
   mx.lock();
   QString t;
   t.setNum((int)b);
   insertIntoCache("Bold", t);
   mx.unlock();
};

void XmlDom::setItalic(bool b){
   mx.lock();
   QString t;
   t.setNum((int)b);
   insertIntoCache("Italic", t);
   mx.unlock();
};

void XmlDom::setUnderline(bool b){
   mx.lock();
   QString t;
   t.setNum((int)b);
   insertIntoCache("Underl", t);
   mx.unlock();
};

void XmlDom::setStrikeout(bool b){
   mx.lock();
   QString t;
   t.setNum((int)b);
   insertIntoCache("Strike", t);
   mx.unlock();
};

void XmlDom::setBackcolor(QColor c){
   mx.lock();
   QString r;
   QString g;
   QString b;
   r.setNum(c.red()) + ",";
   g.setNum(c.green()) +",";
   b.setNum(c.blue());
   insertIntoCache("Backcolor",r+g+b);
   mx.unlock();
};

void XmlDom::setForecolor(QColor c){
   mx.lock();
   QString r;
   QString g;
   QString b;
   r.setNum(c.red()) + ",";
   g.setNum(c.green()) +",";
   b.setNum(c.blue());
   insertIntoCache("Forecolor",r+g+b);
   mx.unlock();
};

void XmlDom::setFramecolor(QColor c){
   mx.lock();
   QString r;
   QString g;
   QString b;
   r.setNum(c.red()) + ",";
   g.setNum(c.green()) +",";
   b.setNum(c.blue());
   insertIntoCache("Framecolor",r+g+b);
   mx.unlock();
};

void XmlDom::setColID(int n){
   mx.lock();
   QString t;
   t.setNum(n);
   insertIntoCache("ID", t);
   mx.unlock();
};

void XmlDom::AddColAttribute(QDomElement c){
   c.setAttribute("ID",         readFromCache("ID"));
   c.setAttribute("Align",      readFromCache("Align"));
   c.setAttribute("Font",       readFromCache("Font"));
   c.setAttribute("Size",       readFromCache("Size"));
   c.setAttribute("Bold",       readFromCache("Bold"));
   c.setAttribute("Italic",     readFromCache("Italic"));
   c.setAttribute("Underl",     readFromCache("Underl"));
   c.setAttribute("Strike",     readFromCache("Strike"));
   c.setAttribute("Backcolor",  readFromCache("Backcolor"));
   c.setAttribute("Forecolor",  readFromCache("Forecolor"));
   c.setAttribute("Framecolor", readFromCache("Framcecolor"));
   c.setAttribute("Width",      readFromCache("Width"));
};

void XmlDom::AddRowAttribute(QDomElement r){
   r.setAttribute("Height",     readFromCache("RowHeight"));
   r.setAttribute("ID",         readFromCache("RowID"));
   r.setAttribute("Pos",        readFromCache("RowPos"));
};

bool XmlDom::AddTable(QString sTabName){
   if(findElement(sTabName, "", "", 0).isNull()){
      mx.lock();
      QDomElement e = createElement(sTabName);
		mainE.appendChild(e);
		mx.unlock(); 
		return true;  
   }
   return false;
};

bool XmlDom::AddRow(QString sTabName, int nRowID){
   QDomElement t = findElement(sTabName, "","", 0);
   mx.lock();
   QString rn = GetRowName(nRowID);
   if(t.isNull()){
      t = createElement(sTabName);
      AddRowAttribute(t);
      mainE.appendChild(t);
   }
   mx.unlock();
   QDomElement r = findElement(sTabName, rn, "", 1);
   mx.lock();
   if(r.isNull()){
      r = createElement(rn);
      t.appendChild(r);
      mx.unlock();
      return true;
   }
   mx.unlock();
   return false;
};

bool XmlDom::AddCol(QString sTabName, int nRowID, int nColID){
   QDomElement t = findElement(sTabName, "", "", 0);
   mx.lock();
   QString rn = GetRowName(nRowID);
   QString cn = GetColName(nColID);
   if(t.isNull()){
      t = createElement(sTabName);
      mainE.appendChild(t);
   }
   mx.unlock();
   QDomElement r = findElement(sTabName, rn, "", 1);
   mx.lock();
   if(r.isNull()){
      r = createElement(rn);
      AddRowAttribute(r);
      t.appendChild(r);
   }
   mx.unlock();
   QDomElement c = findElement(sTabName, rn, cn, 2);
   mx.lock();
   if(c.isNull()){
      c = createElement(cn);
      AddColAttribute(c);
      r.appendChild(c);
      mx.unlock();
      return true;
   }
   mx.unlock();
   return false;
};

QString XmlDom::GetRowName(int nRowID){
   QString t;
   t.setNum(nRowID);
   return "Row" + t;
};

QString XmlDom::GetColName(int nColID){
   QString t;
   t.setNum(nColID);
   return "Col" + t;
};

//Range: 0 -> Table
//Range: 1 -> Row
//Range: 2 -> Column
QDomElement XmlDom::findElement(QString sTab, QString sRow, QString sColumn, int Range){
    mx.lock();
    QDomElement res;
    if(Range == 0){
           mx.unlock();
           return mainE.firstChildElement(sTab);
    }
    if(Range == 1){
           QDomElement t = mainE.firstChildElement(sTab);
           if(t.isNull()){
              mx.unlock();
              return res;
           }
           mx.unlock();
           return t.firstChildElement(sRow);
    }
    if(Range == 2){
           QDomElement t = mainE.firstChildElement(sTab);
           if(t.isNull()){
              mx.unlock();
              return res;
           }
           QDomElement r = t.firstChildElement(sRow);
           if(r.isNull()){
              mx.unlock();
              return res;
           }
           mx.unlock();
           return r.firstChildElement(sColumn);
    }
    mx.unlock();
    return res;
};

XmlDom::XmlDom(QApplication *papp, QString sActFile, QString sApplication){
    mx.lock();
   
	 app = papp;
    ActXmlFile = app->applicationDirPath();
    QStringList ls = sActFile.split("/");
    ActXmlFile += "/" + ls[ls.count()-1] + ".xml";
    QFile fil(ActXmlFile);
    if(!fil.exists()){
      fil.open(QIODevice::WriteOnly);
      QTextStream out(&fil);
      out.setCodec("UTF-8");
      out << "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
		    << "<!DOCTYPE TEdit32>\n"
          << "<TEdit32 version=\"1.0\" >\n</TEdit32>";
      fil.close();
   }
   fil.open(QIODevice::ReadOnly);
   setContent(&fil);
   fil.close();
   mainE = firstChildElement(sApplication);
	 
   mx.unlock();
};

Auf diese Weise taste ich mich nach vorne, und hoffe, dass am Ende kein Thread den Anderen anfrisst [/code]
So mal meine Zwischenlösung.

[upsala]

Nur mal so nebenbei, da du jede Menge locks und unlocks verwendest: Schau dir doch mal die Klasse QMutexLocker an. Würde dir eine Menge erleichtern.

Einfach am Anfang jeder Funktion folgendes verwenden:

Code: Alles auswählen

QMutexLocker locker(&mx);

[Querdenker]

Hi,

da hast Du Recht. Aber die Klasse geht ja noch weiter. Sie hat auch Signals und Slots. Da stört diese Klasse und ich muss gezielt mit dem Mutex arbeiten.
Im Prinzip macht diese Klasse ja auch nichts anderes als das, was ich hier einstreue

Wichtig ist mir, ob ich mit dem Argument richtig liege, dass wenn der ausführende Teil des Ganzen threadsafe ist (das ist auch der Teil der definitiv Daten bewegt), dass ich mir über race conditions nun keine Sorgen machen muss.

Hätte ich gleich die Header dazu gemacht, wäre Dir das bestimmt klar gewesen:

Code: Alles auswählen

#ifndef mDOM_H
#define mDOM_H
#include <QObject>
#include <QMap>
#include <QThreadStorage>
#include <QDomDocument>
#include <QMutex>

class QString;
class QFile;
class QDomElement;
class QApplication;
class QColor;
class QDomNamedNodeMap;

class XmlDom : public QObject {
    Q_OBJECT
   
private:
    QDomDocument             doc;
    QString                  ActXmlFile;
    QApplication             *app;
    QMutex                   mx;
    QDomElement              mainE;
    
    //Templates
    QThreadStorage<QMap<QString, QString>*> cache;
    
	 //private Functions 
    QDomElement findElement (QString, QString, QString, int);
    QString GetRowName      (int);
    QString GetColName      (int);
    void AddRowAttribute    (QDomElement);
    void AddColAttribute    (QDomElement);
    void insertIntoCache    (QString, QString);
    void removeFromCache    (QString);
    QString readFromCache   (QString);
    
public slots:
    //void changeRowAttr (QString, int, const QDomAttr &attr);
    //void changeColAttr (QString, int, int, const QDomAttr &attr);
    void renameTable        (QString, QString);
    void delTable      		 (QString);
    void delRow        		 (QString, int);
    void delCol        		 (QString, int, int);
    
signals:
    void docloaded     		 ();
    void docsaved      		 ();
    void docerror      		 (QString, int , int);
    void rowerror        	 (QString, int);
    void colerror      		 (QString, int, int);
    //void rowchanged    		 (QString, int);
    //void colchanged       	 (QString, int, int);
    void rowremoved         (QString, int);
    void colremoved    		 (QString, int, int);
    void rowadded      		 (QString, int);
    void coladded      		 (QString, int, int);
    void docbeforeload 		 ();
    void docbeforesave 		 ();
    
public:
    XmlDom             		 (QApplication*, QString, QString);
    bool AddTable      		 (QString);
    bool AddRow        		 (QString, int);
    bool AddCol        		 (QString, int, int);
    void setRowID      		 (int);
    void setRowPos     		 (int);
    void setRowHeight  		 (int);
    void setAlign      		 (int);
    void setWidth      		 (int);
    void setFont       		 (QString);
    void setSize       		 (int);
    void setBold       		 (bool);
    void setItalic     		 (bool);
    void setUnderline  		 (bool);
    void setStrikeout  		 (bool);
    void setBackcolor  		 (QColor);
    void setForecolor  		 (QColor);
    void setFramecolor 		 (QColor);
    void setColID      		 (int);
    bool isTableExist  		 (QString);
    bool isRowExist   		 (QString, int);
    bool isColExist   		 (QString, int, int);
    void saveDOM      		 ();
    QDomNamedNodeMap *GetRowAttr (QString, int);
    QDomNamedNodeMap *GetColAttr (QString, int, int);
    bool SetRowAttr(QString, int, const QDomNamedNodeMap &attr);
	 bool SetColAttr(QString, int, int, const QDomNamedNodeMap &attr); 
};
#endif