! Threads ! *augenverdreh*

Hallo Leute!

Versuche mich an einer Thread-Klasse.

Die wichtigen Elemente sehen so aus:

#include <windows.h>
#include <process.h>
#include <stdio.h>

#define NewThread _beginthread
#define EndThread _endthread

class CThread
{
    private :
        unsigned long   m_ulThreadNo;
        DWORD       m_dwIntervall;
        int     m_iPrioritaet;
        UINT        m_uiStackGroesse;

    public :
        bool        m_bLaueft;

        static void ThreadSteuerung(void* Parameter);   // Statische Steuerungsfunktion, die alle CThread-Objekte koordiniert

        CThread();          // Konstruktor
        virtual ~CThread();     // Destruktor

        bool    ThreadStarten();
        void    ThreadStoppen();

        virtual void AusfuehrungsSchleife();
};

// Konstruktor
CThread::CThread()
{
    m_ulThreadNo = -1;
    m_dwIntervall = 0;
    m_iPrioritaet = 0;      // Standardmäßig wird die Priorität vom Prozess übernommen
    m_uiStackGroesse = 0;
    m_bLaueft = false;
}

// Destruktor
CThread::~CThread()
{
    //ThreadStoppen();
}

// Thread-Ausführung
bool CThread::ThreadStarten()
{
    m_ulThreadNo = NewThread(ThreadSteuerung, m_uiStackGroesse, (void*)this);
    if(m_ulThreadNo > 0)
    {
        m_bLaueft = true;
        return true;
    }
    else return false;
}
void CThread::ThreadStoppen()
{
    m_bLaueft = false;
}

// Thread-Schleife
void CThread::AusfuehrungsSchleife()
{
    printf("*");
    // Hier verrichtet die abgeleitete Klasse dann ihre Arbeit
}

// STATISCH !!
void CThread::ThreadSteuerung(void* Parameter)
{
    CThread* AktuellerThread = (CThread*)Parameter;
    while(AktuellerThread->m_bLaueft)
    {
        AktuellerThread->AusfuehrungsSchleife();
        Sleep(AktuellerThread->m_dwIntervall);
    }
    /*if(AktuellerThread->m_ulThreadNo > 0)*/ EndThread();
}

Ausser dieser habe ich noch 2 abgeleitete Klassen, die halt nur AusfuehrungsSchleife überschreiben und statt * dann - o.ä. ausgeben.

Im Main-Teil kreiere ich 1 Thread-Objekt, und jeweils 1 der 2 abgeleiteten Klassen, starte sie mit unterschiedlichen Intervallen.
Das geht auch sehr schön. Jeder printet laut seinem Intervall seine Zeichen.

Im Main-Teil habe ich dann folgende Abfrage:

switch(Eingabe)
            {
                case 1 :
                    if(Ott != NULL)
                    {
                        /*printf("1ster STOP\n");
                        Ott->ThreadStoppen();
                        delete Ott;
                        Ott = NULL;
                        break;*/
                    }
                case 2 :
                    if(OO != NULL)
                    {
                        /*printf("2ster STOP");
                        OO->ThreadStoppen();
                        delete OO;
                        OO = NULL;
                        break;*/
                    }
                case 3 :
                    if(O2 != NULL)
                    {
                        printf("3ster STOP");
                        O2->ThreadStoppen();
                        delete O2;
                        O2 = NULL;
                        break;
                    }
                case 0 :
                    if(Ott != NULL)
                    {
                        printf("1ster STOP");
                        Ott->ThreadStoppen();
                        delete Ott;
                        Ott = NULL;
                    }
                    if(OO != NULL)
                    {
                        printf("2ster STOP");
                        OO->ThreadStoppen();
                        delete OO;
                        OO = NULL;
                    }
                    if(O2 != NULL)
                    {
                        printf("3ster STOP");
                        O2->ThreadStoppen();
                        delete O2;
                        O2 = NULL;
                    }
                    ThreadRunning = false;
                    break;
            }

Jetzt zu meinem Problem. Wenn ich mit '0' ALLE Threads auf einmal beende, gibt's keine Probleme. Nur wenn ich einen einzelnen beenden will (mit '1' - '3'), kommt unmittelbar nach Beendigung eine Speicherverletzung. Ich frage mich halt, woran das liegt. Es muß wohl entweder am Destruktor oder an meiner statischen Kontroll-Funktion liegen.
Sieht jemand vielleicht auf Anhieb den (Denk-)Fehler??

Vielen Dank auf jeden Fall für's Anschauen!

Sarge

P.S.: Es sind einige Sachen jetzt weg- / auskommentiert, das habe ich nur gemacht um den Fehler besser einzugrenzen. Normalerweise gehören sie halt dahin.

P.P.S.: Ott ist ein CThread. OO und O2 sind jeweils Objekte einer abgeleiteten Klasse.

RenéG

Sicherlich, DenkFehler

void CThread::ThreadSteuerung(void* Parameter)
{
    CThread* AktuellerThread = (CThread*)Parameter;
    while(AktuellerThread->m_bLaueft)  // Zeiger ist ungültig, wenn Parameter abgefragt wird!
    {
        AktuellerThread->AusfuehrungsSchleife();
        Sleep(AktuellerThread->m_dwIntervall);
    }
    /*if(AktuellerThread->m_ulThreadNo > 0)*/ EndThread();
}

Original erstellt von RenéG:
Zeiger ist ungültig, wenn Parameter abgefragt wird!

Hmmm... das hab' ich jetzt nicht so ganz verstanden...

Wie könnte man es denn stattdessen machen?
Was ist überhaupt das Problem?

Wenn Windows die Callback aufruft mit dem this-Zeiger, und dieser ungültig wird?! Hmmm...?!?

Aber wenigstens weiß ich jetzt wo ich suchen muß...

Thanks!

Dann liegt das eigentliche Problem wohl im Destruktor. Daß sich die Ausführung noch in der ThreadSteuerungs-Funktion befindet (und m_bLaueft abgefragt wird z.B.), aber mit Eintritt in den Destruktor der this-Zeiger nicht mehr existiert, richtig!?
Lasse ich nämlich delete OO etc. weg funzt es.

Wie kann ich mir da helfen?

CThread* AktuellerThread = (CThread*)Parameter;
if(AktuellerThread)

Original erstellt von <klaus>:
CThread AktuellerThread = (CThread)Parameter;
if(AktuellerThread)**

Das hilft mir nicht weiter.
Sonst müsste ich in der while immer diese Abfrage machen.
Und dann funzt es trotzdem nicht!

Marc++us

Das ist nicht so einfach wie es aussieht... im Grunde mußt Du Dein delete synchronisieren mit dem tatsächlichen Threadende.

Mein Vorschlag wäre, daß Du am Ende von Threadsteuerung() ein delete this; (bzw. hier ein delete AktuellerThread; ) einfügst - damit löschst Du dann am Ende der Schleife den Speicher des Threads. Denn nur der Thread weiß, wann er den Speicher nicht mehr braucht. Danach darfst Du natürlich auf keinerlei Member mehr zugreifen!

Um außerdem zu verhindern, daß weitere Leute den Destruktor aufrufen - z.B. aus dem Hauptprogramm heraus - muß der Destruktor dann private (bzw. falls man Klassen von CThread ableiten kann protected) sein, so daß er nicht außerhalb der Klasse aufgerufen werden kann.

Ein weiterer Ansatz für Dein Problem ist, eine echte Methode zur Prüfung des Threads zu spendieren:

class CThread
{
...
public:
   void WaitForExit()
   {  // mußt noch mal die Params checken... das habe ich schon
      // lange nicht mehr gemacht und kann gerade nicht in der
      // Firma in meine Threadklasse reinsehen :)
      ::WaitForSingleObject(m_ulThreadNo);
   }
};

Dann sieht ein Ende so aus:

O2->ThreadStoppen();
O2->WaitForExit();
delete O2;

Jau, sauber! Genau das hab' ich jetzt endlich rausgefunden! (Also worin das Problem besteht)
Durch Testen.

Und zwar wenn der Destruktor schon durchrasselt, während in der STATIC noch AktuellerThread->m_bLaueft o.ä. abgefragt wird. Das meinte RenéG wohl, der this-Pointer (hier: Parameter) wird ungültig.

Ich hab' das herausgefunden, indem ich am Ende des Destruktors noch ein Sleep(m_dwIntervall + 1000); angefügt habe.
Dann funzt es!
Allerdings ist das eine sehr "gebastelte" Lösung, und wenn mein Prog später (aus welchen Gründen auch immer) DAUERND schnell-lebige Threads erstellt und vernichtet, wahrscheinlich sehr performanzverschwendend...

Jetzt zu Deinen Hinweisen:

Mein Vorschlag wäre, daß Du am Ende von Threadsteuerung() ein delete this; (bzw. hier ein delete AktuellerThread; ) einfügst - damit löschst Du dann am Ende der Schleife den Speicher des Threads. Denn nur der Thread weiß, wann er den Speicher nicht mehr braucht. Danach darfst Du natürlich auf keinerlei Member mehr zugreifen!

Um außerdem zu verhindern, daß weitere Leute den Destruktor aufrufen - z.B. aus dem Hauptprogramm heraus - muß der Destruktor dann private (bzw. falls man Klassen von CThread ableiten kann protected) sein, so daß er nicht außerhalb der Klasse aufgerufen werden kann.

Okay. Später MUSS man Klassen von CThread ableiten, sie soll abstrakt werden (abstrakte Methode AusfuehrungsSchleife()), also Destruktor = protected. Gut, begriffen.

Ein weiterer Ansatz für Dein Problem ist, eine echte Methode zur Prüfung des Threads zu spendieren:

class CThread
{
...
public:
   void WaitForExit()
   {  // mußt noch mal die Params checken... das habe ich schon
      // lange nicht mehr gemacht und kann gerade nicht in der
      // Firma in meine Threadklasse reinsehen :)
      ::WaitForSingleObject(m_ulThreadNo);
   }
};

Dann sieht ein Ende so aus:

O2->ThreadStoppen();
O2->WaitForExit();
delete O2;

Okay. WaitForSingleObject() bekommt auch einen Timeout. Da tue ich dann meine Intervall-Länge rein, richtig?! Oder INFINITE?

Das blöde ist, daß dann weiterer Code ausserhalb aufgerufen werden muß.
Im Prinzip kann ich doch das WaitForExit() dann in den Destruktor schmeißen!?

Also momentan bin ich ziemlich verwirrt, ich probier's aber weiter...

DANKE!

Sarge

Marc++us

Hm ja, das WaitForSingleObject kann auch in den Dtor rein.

INFINITE als Parameter ist gefährlich, weil... kannst Du Dir ja denken.

Auf der anderen Seite kann jede Zeit dazu führen, daß Du zu kurz wartest. So sonderlich genau ist die Zeitberechnung auf Desktop-OSen nicht! Als Wartezeit ist wohl ein Vielfaches Deiner Intervallzeit angemessen.

Darf ich gleich noch 2 Tipps geben:

Was machst Du, wenn die Verarbeitungszeit von AusfuehrungsSchleife() so groß ist, daß Dein folgendes Sleep-Intervall "angeknabbert" wird? Du willst doch z.B. alle 150 msecs eine Funktion starten. Wenn diese aber nun 40msecs braucht... dann dürfte im Sleep nur 110 msecs gewartet werden.

Und zudem wirst Du feststellen, daß Deine Beendigung des Threads über _endthread zu hart ist. Angenommen, in der Ausfuehrungsfunktion ist eine Datei offen... _endthread schießt den Thread einfach ab, Datei nicht geschlossen, Datenverlust, Peng.

Spendiere Deiner Klasse noch zwei virtual Methoden:

virtual void AusfuehrungsSchleife();
virtual void AusfuehrungStart();
virtual void AusfuehrungEnde();

und ändere die Ausführungsfunktion um in:

CThread* AktuellerThread = (CThread*)Parameter;
    AktuellerThread->AusfuehrungStart();
    while(AktuellerThread->m_bLaueft)
    {
        AktuellerThread->AusfuehrungsSchleife();
        Sleep(AktuellerThread->m_dwIntervall);
    }
    AktuellerThread->AusfuehrungEnde();
    EndThread();

Dann kann die abgeleitete Klasse nämlich noch sauber aufräumen.

Mhhhhh...

Jetzt hab' ich mich bislang sooo schön um die lustigen C++ Dinge wie pure virtuals etc. rumgedrückt, und für Threads brauch' ich'se... *grummel*

Ich bekomm' es nicht mal vernünftig hin, daß man von abgeleitete Klassen den Destruktor NICHT aufrufen kann (mit delete). Bzw. wenn, sind die public-Methoden die in CThread definiert sind nicht mehr aufrufbar. Irgendwie blick' ich das mit den Klassen-Vererbungen in C++ wohl noch nicht so...

Original erstellt von Marc++us:
**Hm ja, das WaitForSingleObject kann auch in den Dtor rein.

INFINITE als Parameter ist gefährlich, weil... kannst Du Dir ja denken. :)**

Okay. Ja, aber "rein theoretisch" müsste es funzen. Es sei denn der Thread hat sich abgeschossen. Und das kann doch eigentlich nur durch fehlerhafte Speicherzugriffe passieren (RAM übertaktet etc.), oder?!

Auf der anderen Seite kann jede Zeit dazu führen, daß Du zu kurz wartest. So sonderlich genau ist die Zeitberechnung auf Desktop-OSen nicht! Als Wartezeit ist wohl ein Vielfaches Deiner Intervallzeit angemessen.

Meinst Du echt?!? Ein VIELFACHES????

Darf ich gleich noch 2 Tipps geben:

Sicher!! Nur deswegen poste ich hier...! *freu*

Was machst Du, wenn die Verarbeitungszeit von AusfuehrungsSchleife() so groß ist, daß Dein folgendes Sleep-Intervall "angeknabbert" wird? Du willst doch z.B. alle 150 msecs eine Funktion starten. Wenn diese aber nun 40msecs braucht... dann dürfte im Sleep nur 110 msecs gewartet werden.

Ja, schon klar. Und noch viel interessanter ist die Frage, was passiert, wenn die Ausführung weitaus länger dauert, als das eigentliche Intervall. Und das wird es später wohl. Der Thread müsste also selbst überprüfen, ob er noch "Zeit übrig" hat, und wenn nicht, die Abarbeitung an einen anderen (wartenden) Thread übergeben...
Echt knifflig.

Und zudem wirst Du feststellen, daß Deine Beendigung des Threads über _endthread zu hart ist. Angenommen, in der Ausfuehrungsfunktion ist eine Datei offen... _endthread schießt den Thread einfach ab, Datei nicht geschlossen, Datenverlust, Peng.

Spendiere Deiner Klasse noch zwei virtual Methoden:
virtual void AusfuehrungsSchleife();
virtual void AusfuehrungStart();
virtual void AusfuehrungEnde();

Ja, okay. Verstehe. Init()- und De-Init()-Methoden für den Thread also. Okay.
Das hört sich gut an.

Mhhhh... unter Java ist es soooo viel einfacher!
Naja, mal sehen, da werde ich mich auf jeden Fall noch weiter mit beschäftigen.

Danke mal wieder!

Sarge