VCL und Threadsicherheit

CppDude

Hallo, bin neu hier - daher erstmal ein paar Infos:
- Hobbyprogrammierer: PHP, C++, ASM, JavaScript (GreaseMonkey)
- Entwicklungsumgebung: Borland C++ Builder 6

Ich arbeite nun schon seit ein paar Jahren mit dem 'TThread' Object vom C++ Builder und stoße immer wieder auf die Probleme mit der Threadsicherheit bezüglich der Verwendung der VCL und Multithreaded-Anwendungen.

Daher wollte ich nun hier mal das Grundgerüst posten und euch fragen ob ich soweit alles richtig mache:
(Die unwichtigen Dinge habe ich mal weggelassen, weil so schon genug ist)

Edit: Code angepasst
- bezüglich des 'delete' vom NULL-Zeiger;
- 'OnTerminate()'-Methode in den Threads hinzugefügt

// UnitMain.cpp
// Hauptformular der Anwendung - TMain
//---------------------------------------------------------------------------
#include <vcl.h>        // Visual Command Library (VCL)
#pragma hdrstop
//---------------------------------------------------------------------------
#include "UnitMain.h"
#include "ThreadTime.h"      // TTrdTime
#include "ThreadExecute.h"   // TTrdExecute
//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
//---------------------------------------------------------------------------
TMain *Main;

// ##########################################################################      ###  Konstruktor
// ### Konstruktor - deklarierte Variablen initialisieren                 ###
// ##########################################################################

//---------------------------------------------------------------------------
__fastcall TMain::TMain(TComponent* Owner)
        : TForm(Owner)
{
  // ini File laden
  iniLoad();

  // Thread starten
  hTrdTime = new TTrdTime();
}
//---------------------------------------------------------------------------

// ##########################################################################      ###  Destruktor
// ### Destruktor - Speicher aller initialisierten Variablen freigeben    ###
// ##########################################################################

//---------------------------------------------------------------------------
__fastcall TMain::~TMain()
{
  // prüfen ob TTrdTime noch läuft
  if (hTrdTime)
  {
    // Thread beenden und Speicher freigeben
    hTrdTime->Terminate();

    // Zeiger-Zuweisung löschen
    hTrdTime = NULL;
  }

  // prüfen ob TTrdTime noch läuft
  if (hTrdExecute)
  {
    // Thread beenden und Speicher freigeben
    hTrdExecute->Terminate();

    // Zeiger-Zuweisung löschen
    hTrdExecute = NULL;
  }

  // ini File speichern
  iniSave();
}
//---------------------------------------------------------------------------

// ##########################################################################      ### ini-File
// ### ini File Laden / Speichern                                         ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TMain::iniLoad()
{
  // ini laden ...
}
//---------------------------------------------------------------------------
void __fastcall TMain::iniSave()
{
  // ini speichern ...
}
//---------------------------------------------------------------------------

// ##########################################################################      ### ThreadTime Status
// ### ThreadTime Status - <AN> / <AUS>, ErrorText                        ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TMain::TrdTimeStatus(int iStatus, String ErrorText)
{
  if (iStatus == 1) // <AN>
  {
    // Thread ist AN
  }
  else if (iStatus == 0) // <AUS>
  {
    // Thread ist AUS
  }
  else if (iStatus == 2) // ExecuteThread starten
  {
    // Thread starten
    hTrdExecute = new TTrdExecute();
  }

  //-------------------------------------------------------------------------

  if (ErrorText != "")
  {
    Application->MessageBox( String(ErrorText).c_str(), "Fehler im Time-Thread:", 0+48 );
  }

  //-------------------------------------------------------------------------
}
//---------------------------------------------------------------------------

// ##########################################################################      ### ThreadExecute Status
// ### ThreadExecute Status - <AN> / <AUS>, ErrorText                     ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TMain::TrdExecuteStatus(int iStatus, String ErrorText)
{
  if (iStatus == 1) // <AN>
  {
    // Thread ist AN
  }
  else if (iStatus == 0) // <AUS>
  {
    // prüfen ob TTrdTime noch läuft
    if (hTrdTime)
    {
      // Thread beenden und Speicher freigeben
      hTrdTime->Terminate();

      // Zeiger-Zuweisung löschen
      hTrdTime = NULL;
    }
  }

  //-------------------------------------------------------------------------

  if (ErrorText != "")
  {
    Application->MessageBox( String(ErrorText).c_str(), "Fehler im Execute-Thread:", 0+48 );
  }

  //-------------------------------------------------------------------------
}
//---------------------------------------------------------------------------

// UnitMain.h
// Header vom Hauptformular - TMain
//---------------------------------------------------------------------------
#ifndef UnitMainH
#define UnitMainH
//---------------------------------------------------------------------------
#include <Classes.hpp>
#include <Controls.hpp>
#include <StdCtrls.hpp>
#include <Forms.hpp>
#include <ExtCtrls.hpp>
#include <ComCtrls.hpp>
//---------------------------------------------------------------------------
class TTrdTime;    // Forward Deklaration des Threads - TTrdTime
class TTrdExecute; // Forward Deklaration des Threads - TTrdExecute
//---------------------------------------------------------------------------
class TMain : public TForm
{
  //-------------------------------------------------------------------------
  __published:	// Von der IDE verwaltete Komponenten

      // TPanel zum Anzeigen der Laufzeit aus TTrdTime
      // Format: 00:00:00 - hh:mm:ss
      TPanel *Zeit;

  //-------------------------------------------------------------------------
  private:	// Anwender-Deklarationen

    // ini-File laden/speichern
    void __fastcall iniLoad();
    void __fastcall iniSave();

    // Thread Handles
    TTrdTime    *hTrdTime;
    TTrdExecute *hTrdExecute;

  //-------------------------------------------------------------------------
  public:		// Anwender-Deklarationen

    // ThreadTime Status - <AN> / <AUS>, ErrorText
    void __fastcall TrdTimeStatus(int iStatus, String ErrorText);

    // ThreadExecute Status - <AN> / <AUS>, ErrorText
    void __fastcall TrdExecuteStatus(int iStatus, String ErrorText);

    //-----------------------------------------------------------------------

    // Konstruktor - deklarierte Variablen initialisieren
    __fastcall TMain(TComponent* Owner);

    // Destruktor - Speicher aller initialisierten Variablen freigeben
    __fastcall ~TMain();

    //-----------------------------------------------------------------------
};
//---------------------------------------------------------------------------
extern PACKAGE TMain *Main;
//---------------------------------------------------------------------------
#endif

// ThreadTime.cpp
// Time Thread - TTrdTime
//---------------------------------------------------------------------------
#include <vcl.h>        // Visual Command Library (VCL)
#pragma hdrstop
//---------------------------------------------------------------------------
#include "ThreadTime.h"
#include "UnitMain.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
//---------------------------------------------------------------------------

// ##########################################################################      ### Konstruktor
// ### Konstruktor - deklarierte Variablen initialisieren                 ###
// ##########################################################################

//---------------------------------------------------------------------------
__fastcall TTrdTime::TTrdTime(void)
: TThread(False)
{
  // Thread-Speicher freigeben, wenn 'Terminated == true'
  // Aufruf durch 'Terminate()'
  FreeOnTerminate = true;

  // 'OnTerminate()'-Methode festlegen
  OnTerminate = ThreadOnTerminate;

  // Zeiger auf das Main Formular
  MainForm = Main;

  // Pause um Prozessorauslastung nicht auf 100% zu bringen
  Pause = 5; // 5 ms

  // ExceptionText zur Kommunikation mit dem Main Form
  ExceptionText = "";

  // Variablen für die Zeit-Anzeige
  H = "", M = "", S = "";
}
//---------------------------------------------------------------------------

// ##########################################################################      ### Destruktor
// ### Destruktor - Speicher aller initalisierten Variablen freigeben     ###
// ##########################################################################

//---------------------------------------------------------------------------
__fastcall TTrdTime::~TTrdTime()
{
  try
  {
    // Zeiger auf das Main Formular freigeben
    MainForm = NULL;
  }
  catch (...) {}
}
//---------------------------------------------------------------------------

// ##########################################################################      ###
// ### Thread Execute Funktion                                            ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TTrdTime::Execute()
{
  try
  {
    // Status des Threads auf <AN> setzen
    iStatus = 1;

    // Main Formular informieren, dass Thread <AN> ist
    Synchronize(SendStatus);

    // Startzeitpunkt ermitteln
    int TimeStart = ::GetTickCount();

    // solange der Thread nicht vom Main Formular aus beendet wird
    while (!Terminated)
    {
      // Zeitberechnung
      // ...

      // aktuelle Laufzeit anzeigen
      // (wird aller 1000 ms aufgerufen)
      if ( ::GetTickCount() - TimeStart >= 1000)
      {
        Synchronize(ShowZeit);

        // Startzeitpunkt reset
        TimeStart = ::GetTickCount();
      }

      // Pause um Prozessorauslastung nicht auf 100% zu treiben
      Sleep(Pause);
    }
    // end: while (!Terminated)
  }
  catch (Exception *E)
  {
    // ExceptionText enthält die Exception (Fehlernachricht)
    ExceptionText = E->Message;

    // Main Form über Status des Threads informieren
    Synchronize(SendStatus);

    // Thread-Speicher freigeben
    Terminate();
  }
}
//---------------------------------------------------------------------------

// ##########################################################################      ### Thread 'OnTerminate()'-Methode  
// ### Thread 'OnTerminate()'-Methode                                     ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TTrdTime::ThreadOnTerminate(TObject *Sender)
{
  /* hier kein 'Synchronize()' verwenden - VCL-Haupthread-Kontext */

  // Status des Threads auf <AUS> setzen
  iStatus = 0;

  // Main Form informieren, dass Thread <AUS> ist
  SendStatus();
}
//---------------------------------------------------------------------------

// ##########################################################################      ### Thread Status
// ### Thread Status - <AN> / <AUS>, ExceptionText                        ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TTrdTime::SendStatus(void)
{
  // Status des Threads: <AN> / <AUS>, ExceptionText
  MainForm->TrdTimeStatus(iStatus, ExceptionText);

  // ExceptionText reseten
  ExceptionText = "";
}
//---------------------------------------------------------------------------

// ##########################################################################      ### Zeit anzeigen
// ### Zeit anzeigen                                                      ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TTrdTime::ShowZeit(void)
{
  // Zeit anzeigen - Format: 00:00:00
  MainForm->Zeit->Caption = H + ":" + M + ":" + S;
}
//---------------------------------------------------------------------------

// ThreadTime.h
// Zeit Thread - TTrdTime - Header
//---------------------------------------------------------------------------
#ifndef ThreadTimeH
#define ThreadTimeH
//---------------------------------------------------------------------------
#include <Classes.hpp>
#include <Controls.hpp>
#include <StdCtrls.hpp>
//---------------------------------------------------------------------------
class TMain; // Forward Deklaration des Main Form
//---------------------------------------------------------------------------
class TTrdTime : public TThread
{
  //-------------------------------------------------------------------------
  private: /* Variablen-Deklarationen und Methoden-Deklarationen */

    // Zeiger auf das Main Formular
    TMain *MainForm;

    // Pause um Prozessorauslastung nicht auf 100% zu bringen
    DWORD Pause;

    // Status des Threads <AN> / <AUS> (<1>/<0>)
    int iStatus;

    // ExceptionText zur Kommunikation mit dem Main Form
    String ExceptionText;

    // Variablen für die Zeit
    String H, M, S;

    //-----------------------------------------------------------------------

    // Thread Status - TrdID, <AN> / <AUS>, StatusText, ExceptionText
    void __fastcall SendStatus(void);

    // Zeit anzeigen
    void __fastcall ShowZeit(void);

  //-------------------------------------------------------------------------
  protected:

    // Thread 'Execute()'-Methode
    virtual void __fastcall Execute(void);

    // Thread 'OnTerminate()'-Methode
    virtual void __fastcall ThreadOnTerminate(TObject *Sender);

  //-------------------------------------------------------------------------
  public: /* Konstruktor - Destruktor */

    // Konstruktor - deklarierte Variablen initalisieren
    __fastcall TTrdTime(void);

    // Destruktor - Speicher aller initalisierten Variablen freigeben
    __fastcall ~TTrdTime();

  //-------------------------------------------------------------------------
};
//---------------------------------------------------------------------------
#endif

// ThreadExecute.cpp
// Execute Thread - TTrdExecute
//---------------------------------------------------------------------------
#include <vcl.h>        // Visual Command Library (VCL)
#pragma hdrstop
//---------------------------------------------------------------------------
#include "ThreadExecute.h"
#include "UnitMain.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
//---------------------------------------------------------------------------

// ##########################################################################      ### Konstruktor
// ### Konstruktor - deklarierte Variablen initialisieren                 ###
// ##########################################################################

//---------------------------------------------------------------------------
__fastcall TTrdExecute::TTrdExecute(void)
: TThread(False)
{
  // Thread-Speicher freigeben, wenn 'Terminated == true'
  // Aufruf durch 'Terminate()'
  FreeOnTerminate = true;

  // 'OnTerminate()'-Methode festlegen
  OnTerminate = ThreadOnTerminate;

  // Zeiger auf das Main Formular
  MainForm = Main;

  // Pause um Prozessorauslastung nicht auf 100% zu bringen
  Pause = 5; // 5 ms

  // ExceptionText zur Kommunikation mit dem Main Form
  ExceptionText = "";
}
//---------------------------------------------------------------------------

// ##########################################################################      ### Destruktor
// ### Destruktor - Speicher aller initalisierten Variablen freigeben     ###
// ##########################################################################

//---------------------------------------------------------------------------
__fastcall TTrdExecute::~TTrdExecute()
{
  try
  {
    // Zeiger auf das Main Formular freigeben
    MainForm = NULL;
  }
  catch (...) {}
}
//---------------------------------------------------------------------------

// ##########################################################################      ###
// ### Thread Execute Funktion                                            ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TTrdExecute::Execute()
{
  try
  {
    // Status des Threads auf <AN> setzen
    iStatus = 1;

    // Main Formular informieren, dass Thread <AN> ist
    Synchronize(SendStatus);

    // solange der Thread nicht vom Main Formular aus beendet wird
    while (!Terminated)
    {
      // Berechnungungen durchführen
      // ...

      // Pause um Prozessorauslastung nicht auf 100% zu treiben
      Sleep(Pause);
    }
    // end: while (!Terminated)
  }
  catch (Exception *E)
  {
    // ExceptionText enthält die Exception (Fehlernachricht)
    ExceptionText = E->Message;

    // Main Form über Status des Threads informieren
    Synchronize(SendStatus);

    // Thread-Speicher freigeben
    Terminate();
  }
}
//---------------------------------------------------------------------------

// ##########################################################################      ### Thread 'OnTerminate()'-Methode  
// ### Thread 'OnTerminate()'-Methode                                     ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TTrdExecute::ThreadOnTerminate(TObject *Sender)
{
  /* hier kein 'Synchronize()' verwenden - VCL-Haupthread-Kontext */

  // Status des Threads auf <AUS> setzen
  iStatus = 0;

  // Main Form informieren, dass Thread <AUS> ist
  SendStatus();
}
//---------------------------------------------------------------------------

// ##########################################################################      ### Thread Status
// ### Thread Status - <AN> / <AUS>, ExceptionText                        ###
// ##########################################################################

//---------------------------------------------------------------------------
void __fastcall TTrdExecute::SendStatus(void)
{
  // Status des Threads: <AN> / <AUS>, ExceptionText
  MainForm->TrdExecuteStatus(iStatus, ExceptionText);

  // ExceptionText reseten
  ExceptionText = "";
}
//---------------------------------------------------------------------------

// ThreadExecute.h
// Execute Thread - TTrdExecute - Header
//---------------------------------------------------------------------------
#ifndef ThreadExecuteH
#define ThreadExecuteH
//---------------------------------------------------------------------------
#include <Classes.hpp>
#include <Controls.hpp>
#include <StdCtrls.hpp>
//---------------------------------------------------------------------------
class TMain; // Forward Deklaration des Main Form
//---------------------------------------------------------------------------
class TTrdExecute : public TThread
{
  //-------------------------------------------------------------------------
  private: /* Variablen-Deklarationen und Methoden-Deklarationen */

    // Zeiger auf das Main Formular
    TMain *MainForm;

    // Pause um Prozessorauslastung nicht auf 100% zu bringen
    DWORD Pause;

    // Status des Threads <AN> / <AUS> (<1>/<0>)
    int iStatus;

    // ExceptionText zur Kommunikation mit dem Main Form
    String ExceptionText;

    //-----------------------------------------------------------------------

    // Thread Status - TrdID, <AN> / <AUS>, StatusText, ExceptionText
    void __fastcall SendStatus(void);

  //-------------------------------------------------------------------------
  protected:

    // Thread 'Execute()'-Methode
    virtual void __fastcall Execute(void);

    // Thread 'OnTerminate()'-Methode
    virtual void __fastcall ThreadOnTerminate(TObject *Sender);

  //-------------------------------------------------------------------------
  public: /* Konstruktor - Destruktor */

    // Konstruktor - deklarierte Variablen initalisieren
    __fastcall TTrdExecute(void);

    // Destruktor - Speicher aller initalisierten Variablen freigeben
    __fastcall ~TTrdExecute();

  //-------------------------------------------------------------------------
};
//---------------------------------------------------------------------------
#endif

Ich hoffe jemand nimmt sich die Zeit und schaut da mal drüber.

Das Programm läuft auf jeden Fall fehlerfrei auf diese Art und Weise,
aber das heißt ja noch lang nicht das ich auch alles richtig mache.

-CppDude

Hallo,

ich habe zwar nicht den kompletten Code durchgeschaut, aber ein paar Dinge sind m.e. grundlegend falsch.

//---------------------------------------------------------------------------
__fastcall TMain::~TMain()
{
  try
  {
    // TTrdTime stoppen
    if (hTrdTime)
    {
      // Thread beenden und Speicher freigeben
      hTrdTime->Terminate();

      // Zeiger-Zuweisung löschen
      hTrdTime = NULL;

      // Speicher freigeben
      delete hTrdTime;
    }

Zuerst 'hTrdTime = NULL' und danach 'delete hTrdTime' geht gar nicht. Wenn der Zeiger NULL ist, dann führt das nachfolgende delete unweigerlich zu einer Exception.
Außerdem darf 'delete hTrdTime' nicht aufgerufen werden, wenn im Thread-Objekt 'FreeOnTerminate = true' gesetzt wird.
Wenn 'FreeOnTerminate = true' gesetzt ist, wird die Instance nach Beenden von Execute() automatisch gelöscht und der Speicher freigegeben. Hier darf auf keinen Fall nochmals 'delete hTrdTime' aufgerufen werden.

Falls das Main-Formular über das Beenden der Thread-Instance benachrichtigt werden muss, kann das in TTrdTime::OnTerminate() gemacht werden. Aber bitte beachten, dass OnTerminate() bereits ausserhalb des Threads liegt, deshalb hier kein Synchronize() verwenden. Die Verwendung von Synchronize() außerhalb der Execute()-Methode kann dazu führen, dass das Programm "hängen" bleibt.

CppDude

Hallo, danke für deine Antwort.

j.halder schrieb:

Zuerst 'hTrdTime = NULL' und danach 'delete hTrdTime' geht gar nicht. Wenn der Zeiger NULL ist, dann führt das nachfolgende delete unweigerlich zu einer Exception.

Ich bin auch grad am Überlegen wie ich auf diese 'geniale' Idee gekommen bin.
Das Löschen eines Nullzeigers führt zwar nicht zu einer Exception, da 'delete' dann garnichts ausführt, ist aber trotzdem sinnlos.

j.halder schrieb:

Außerdem darf 'delete hTrdTime' nicht aufgerufen werden, wenn im Thread-Objekt 'FreeOnTerminate = true' gesetzt wird.
Wenn 'FreeOnTerminate = true' gesetzt ist, wird die Instance nach Beenden von Execute() automatisch gelöscht und der Speicher freigegeben. Hier darf auf keinen Fall nochmals 'delete hTrdTime' aufgerufen werden.

Ich denke das war der Teil, der mir nicht bewusst war.

Ich bin davon ausgegangen das ein Aufruf von 'new' zwingend den Aufruf von 'delete' benötigt,
aber das scheint ja dann hier bei den Threads nicht so zu sein, sofern 'FreeOnTerminate = true'.

j.halder schrieb:

Falls das Main-Formular über das Beenden der Thread-Instance benachrichtigt werden muss, kann das in TTrdTime::OnTerminate() gemacht werden. Aber bitte beachten, dass OnTerminate() bereits ausserhalb des Threads liegt, deshalb hier kein Synchronize() verwenden.

Das 'TTrdTime::OnTerminate()' werd ich wohl wirklich mal einbauen... keine Ahnung wieso ich das bisher
noch nicht verwendet habe. Scheint auf jeden Fall sinnvoller zu sein, als meine Variante.

j.halder schrieb:

Die Verwendung von Synchronize() außerhalb der Execute()-Methode kann dazu führen, dass das Programm "hängen" bleibt.

Das ist mir auch neu.
Heißt das etwa auch, dass ein Aufruf von 'Synchronize()' nur aus der Execute()-Methode erfolgen sollte
und andere Methoden, welche über die Execute()-Methode aufgerufen werden, sollten dann darauf verzichten?

Beispiel:
1.) Aufruf der Methode 'test()' über die Execute()-Methode
2.) Aufruf von 'Synchronize()' innerhalb der 'test()-Methode' <<< Wäre das auch falsch?

Ich werd jetzt nochmal durch ein paar Tutorials zum Thema TThread lesen und mal schauen was ich noch
so finden kann. Ich danke dir schonmal für deine Hilfe.

-CppDude

DocShoe

j.halder schrieb:

Hallo,

ich habe zwar nicht den kompletten Code durchgeschaut, aber ein paar Dinge sind m.e. grundlegend falsch.
//---------------------------------------------------------------------------
__fastcall TMain::~TMain()
{
  try
  {
    // TTrdTime stoppen
    if (hTrdTime)
    {
      // Thread beenden und Speicher freigeben
      hTrdTime->Terminate();
 
      // Zeiger-Zuweisung löschen
      hTrdTime = NULL;
 
      // Speicher freigeben
      delete hTrdTime;
    }
Zuerst 'hTrdTime = NULL' und danach 'delete hTrdTime' geht gar nicht. Wenn der Zeiger NULL ist, dann führt das nachfolgende delete unweigerlich zu einer Exception.
Außerdem darf 'delete hTrdTime' nicht aufgerufen werden, wenn im Thread-Objekt 'FreeOnTerminate = true' gesetzt wird.
Wenn 'FreeOnTerminate = true' gesetzt ist, wird die Instance nach Beenden von Execute() automatisch gelöscht und der Speicher freigegeben. Hier darf auf keinen Fall nochmals 'delete hTrdTime' aufgerufen werden.

Minus mal Minus gibt Plus

Lustigerweise führen die beiden Fehler zusammen zu korrektem Verhalten. Terminate zusammen mit FreeOnTerminate sorgen dafür, dass das Objekt korrekt abgeräumt wird. Das Setzen von hTrdTime auf NULL mit anschließendem delete hTrdTime sorgt dafür, dass der Speicher nicht zwei Mal freigegeben wird. Ein delete NULL ist unproblematisch, da es einfach nichts macht.

Heißt das etwa auch, dass ein Aufruf von 'Synchronize()' nur aus der Execute()-Methode erfolgen sollte und andere Methoden, welche über die Execute()-Methode aufgerufen werden, sollten dann darauf verzichten?

Nein!
Grundsätzlich gilt:
Solange sich der Thread-Process innerhalb von Execute() oder einer in Execute() aufgerufenen Methode befindet, MUSS Synchronize() verwendet werden.
Befindet sich der Thread-Process außerhalb von Execute() (Konstruktor, Destruktor, OnTerminate()), dann darf Synchronize() NICHT verwendet werden.

In deinem Fall:
Wenn du test() in Execute() aufrufst , dann mußt du auch in test() Synchronize() verwenden.
Falls du test() aber im Konstruktor, Destruktor oder in OnTerminate() aufrufst, darfst du in test() kein Synchronice() verwenden.

Der Thread ist ein eigenständiger Process, aber nur solange er sich innerhalb von Execute() oder einer in Execute() aufgerufenen Methode befindet.
Vor dem Einsprung in Execute() und nach dem Verlassen von Execute() ist der Thread ein Teil des Haupt-Processes (Main).

Synchronice() dient dazu, Thread- und Haupt-Process miteinander zu synchronisieren. Beide Processe laufen parallel. Wenn der Thread nun eine Funktion des Haupt-Processes aufruft, dann muss der Haupt-Process zuerst seine aktuelle Arbeit stoppen, Register, Stack etc. sichern, erst dann kann er die vom Thread aufgerufene Methode ausführen.

Falls Synchronize() außerhalb des Thread-Processes verwendet wird, versucht der Haupt-Process sich mit sich selbst zu synchronisieren. Das kann hundert mal gut gehen (je nachdem, was der Haupt-Process gerade macht), aber wenn es schiefgeht, dann so, dass nichts mehr geht. Und diesen Fehler, der nur sporadisch auftritt, kann man fast nicht lokalisieren.
Ich habe die Erfahrung gemacht, dass so ein Synchronisationsfehler nur auftritt, wenn das Programm eigenständig läuft. Innerhalb der IDE mit Debugger usw. läßt sich das nicht nachvollziehen, da funktioniert es immer.

CppDude

Danke für diese ausführliche Erklärung. Damit hast du mir schonmal sehr weitergeholfen.
Denn so ganz sicher war ich mir bei den Aufrufen vom 'Synchronize()' auch nicht.

Ich habe noch ein weiteres "Problem" bei dem ich mir nicht sicher bin:
1.) Ich rufe aus der 'Execute()'-Methode eine Funktion auf 'Synchronize(Zeichnen)';
2.) Die Funktion erstellt dann ein TPanel:

//---------------------------------------------------------------------------
void __fastcall TTrdExecute::Zeichnen(void)
{
    TPanel *MyPanel;

    MyPanel = new TPanel(MainForm);
    MyPanel->Parent  = MainForm;
    MyPanel->Name    = "MyPanel";
    MyPanel->Left    = 50;
    MyPanel->Top     = 125;
    MyPanel->Width   = 65;
    MyPanel->Height  = 65;
    MyPanel->Caption = "test";
    MyPanel->BevelInner  = bvNone;
    MyPanel->BevelOuter  = bvNone;
    MyPanel->ParentFont  = true;
    MyPanel->Font->Size  = 15;
    MyPanel->ParentColor = true;

    // Funktionsaufruf für 'OnClick()'-Methode zuweisen
    MyPanel->OnClick = MyPanelClick;

    // Zeigerzuweisung löschen
    MyPanel     = NULL;
}
//---------------------------------------------------------------------------

3.) Die 'OnClick()'-Methode wird ja nun aufgerufen, wenn ich den Panel anklicke...
Frage: Brauche ich dann innerhalb dieser 'OnClick()'-Methode das 'Synchronize()' für Funktionsaufrufe?

//---------------------------------------------------------------------------
void __fastcall TTrdExecute::MyPanelClick(TObject *Sender)
{
  // Zeiger auf den Sender zu TPanel casten
  TPanel *MyPanel = dynamic_cast<TPanel*>(Sender);

  if (MyPanel)
  {
    // Panel löschen
    delete MyPanel;
  }

  // Funktionsaufruf MIT oder OHNE 'Synchronize()' ???
  Test();
}
//---------------------------------------------------------------------------

Die nächste Frage wäre außerdem ob das mit dem TPanel und dem Speicher klar geht.
Muss ich mich noch um den Speicher kümmern, oder reicht das 'casten' und das 'delete' aus?

Edit:
Casten vom Panel klappt, aber beim 'delete' gibts ne Zugriffsverletzung.
Wie müsste ich dass denn jetzt lösen, wenn ich das Panel im OnClick löschen will?

DocShoe

In j.halders Posting stecken einige Detailfehler, dazu möchte noch etwas sagen:

j.halder schrieb:

Der Thread ist ein eigenständiger Process, aber nur solange er sich innerhalb von Execute() oder einer in Execute() aufgerufenen Methode befindet.
Vor dem Einsprung in Execute() und nach dem Verlassen von Execute() ist der Thread ein Teil des Haupt-Processes (Main).

Ein Thread ist kein Prozess. Niemals. Ein Thread ist Teil eines Prozesses, aber zwischen Prozess und Thread gibt es himmelweite Unterschiede. Wikipedia liefert da ausführliche Informationen.

j.halder schrieb:

Synchronice() dient dazu, Thread- und Haupt-Process miteinander zu synchronisieren. Beide Processe laufen parallel. Wenn der Thread nun eine Funktion des Haupt-Processes aufruft, dann muss der Haupt-Process zuerst seine aktuelle Arbeit stoppen, Register, Stack etc. sichern, erst dann kann er die vom Thread aufgerufene Methode ausführen.

Für deine Anwendung existiert genau ein Prozess. In diesem Prozess laufen einige Threads, einer davon ist der VCL Hauptthread. So etwas wie Thread-Prozess oder Haupt Prozess existieren nicht, es gibt nur den Prozess.
Der VCL Hauptthread ist für die Abarbeitung der Windows Nachrichten zuständig und steuert die GUI bzw. reagiert auf Benutzereingaben. Du kannst aus einem anderen Thread beliebige Funktionen/Methoden aufrufen, solange du keine GUI Elemente anfasst.

j.halder schrieb:

Solange sich der Thread-Process innerhalb von Execute() oder einer in Execute() aufgerufenen Methode befindet, MUSS Synchronize() verwendet werden.
Befindet sich der Thread-Process außerhalb von Execute() (Konstruktor, Destruktor, OnTerminate()), dann darf Synchronize() NICHT verwendet werden.

Das ist nur halb wahr. Du darfst Synchronize nicht im im VCL Hauptthread aufrufen, weil es zu Deadlocks führen kann, ansonsten existieren keine Einschränken. Es kommt also auf den Thread-Kontext an, nicht auf die Methode, in der Synchronize aufgerfuen wird.

@TE:
Das Problem der Zugriffsverletzung hängt höchstwahrscheinlich damit zusammen, dass du das Objekt, das dir die Windows Nachricht zugeschickt hat, während der Nachrichtenbehandlung löscht. Nach dem Verlassen der MyPanelClick kehrt die CPU in den Code des Aufrufers zurück, der zu diesem Zeitpunkt aber gelöscht wurde, und das führt zu einer Zugriffsverletzung. Der Ablauf der Nachrichtenbehandlung sieht (stark vereinfacht) etwa so aus:

void TObject::Dispatch( void* Message )
{
  // Zauberei hier, in folge dessen irgendwann 
  // MyPanelClick aufgerufen wird.
  if( OnClick ) OnClick( this );

  // hier passiert noch mehr, aber es führt zu einem Zugriffsfehler, weil dieses
  // Objekt in deinem OnClick Handler gelöscht wurde und diese Methode auf
  // Attribute des aktuellen Objekts zugreifen will.
}

Du hast ein Timing Problem, du darfst das Panel im OnClick Handler nicht löschen. Ein Möglichkeit wäre z.B., das Panel zum Löschen markieren und in einem Timer Event alle zum Löschen markierte Elemente tatsächlich zu löschen.

CppDude

Danke für die Erläuterungen DocShoe!
Den Wiki-Artikel werd ich mir aich gleich nochmal genauer durchlesen.

Das mit der Zugriffsverletzung hört sich mit deiner Erklärung auch logisch an.
Werde mal schauen wie ich das löschen vom Panel sonst lösen kann.
Zunächst kann ich ja statt 'delete' auch 'MyPanel->Visible = false' nutzen um den Panel optisch schonmal loszuwerden.
Danach versuche ich dann das ganze TPanel per 'Freigabe' und mit Timer zu löschen.

Edit:
Mit einer Abfrage in der 'Execute()'-Methode und 'bool bDeletePanel' als Variable klappt es ohne zugriffsverletzung.
Danke für dne Tipp und die Erläuterung!

CppDude

Eine Frage bleibt aber immernoch offen:

Wenn ich im Thread ein TPanel erstelle und 'OnClick = MyPanelClick' zuweise,
brauche ich dann 'Synchronize()' für Methoden-Aufrufe aus der 'OnClick()'-Methode heraus?

//--------------------------------------------------------------------------- 
void __fastcall TTrdExecute::MyPanelClick(TObject *Sender) 
{
  // Methoden-Aufruf MIt oder OHNE 'Synchronize()' ???
  // Test();
  // Synchronize(Test); 
}
//---------------------------------------------------------------------------

DocShoe

Hi,

nein, vermutlich brauchst du kein Synchronize . Der Code gehört zwar zu deiner Thread Klasse, wird aber aus VCL Hauptthread aufgerufen. Es sei denn, du erzeugst das Panel dynamisch in einem deiner Threads (und nicht im VCL Hauptthread). Genau genommen habe ich die Frage schon beantwortet:

DocShoe schrieb:

Das ist nur halb wahr. Du darfst Synchronize nicht im im VCL Hauptthread aufrufen, weil es zu Deadlocks führen kann, ansonsten existieren keine Einschränken. Es kommt also auf den Thread-Kontext an, nicht auf die Methode, in der Synchronize aufgerfuen wird.

Du solltest dein Design gut genug kennen, um die Frage zu beantworten.

CppDude

Ja, das TPanel wird dynamisch in einem der Threads erstellt und die 'OnClick()'-Methode wird innerhalb vom Thread zugewiesen.
Code-Beispiel steht auch ein paar Beiträge weiter oben.

Kann ich denn davon ausgehen, dass der Klick selbst vom VCL-Hauptthread gesendet wird,
wenn ich den (im Thread erstellten) Panel anklicke?

DocShoe

Nein, dann brauchst du vermutlich doch die Synchronize Methode.
Nur so aus Neugierde:
Was machen deine Threads eigentlich, dass sie Panels erzeugen und wieder löschen? Hört sich seltsam an...

Edit:
Die Textwand habe ich mir nicht angeguckt. tl;dr

CppDude

Ich bastel grad ne Art Geschicklichkeits-Spiel mit Denkaufgaben, Memory-Aufgaben, Geschicklichkeits-Tests, Schnelligkeitstests usw.
Damit ich nicht 100 verschiedene Sachen vorbereiten muss, erstelle ich das pro Aufgabe dynamisch und mit random im Thread
und nach jeder Aufgabe werden die aufgaben-spezifischen Panel wieder gelöscht.

Das ganze läuft auf Zeit und mit Punktzahl pro Aufgabe + Zeitbonus;
Am Ende gibts dann localen + online Highscore.

---

Kann ich i-wie herausfinden ob ich im VCL-Hauptthread-Kontext arbeite?

DocShoe

Da besteht doch gar keine Notwendigkeit, das in einem Thread zu machen...

Th69

Hallo ihr beiden,

der Click-Event wird auf jeden Fall innerhalb des VCL-Hauptthreads ausgeführt, so daß kein Synchronize nötig ist (egal von welchem Thread aus das Ereignis registriert wurde).

P.S. Sehe ich genauso, daß hier ein Thread unnötig ist -> dies ist doch ein klassischer Fall für einen Timer (und dieser läuft automatisch im VCL-Hauptthread).

DocShoe

Aha, danke für die Info Th69. War mir jetzt so nicht klar, ich bin immer davon ausgegangen, dass der Thread, der ein GUI Element erzeugt, auch dessen Message Queue bedient.

CppDude schrieb:

Kann ich i-wie herausfinden ob ich im VCL-Hauptthread-Kontext arbeite?

Wenn du ein Fenster hast, von dem du weißt, dass es im VCL Hauptthread erzeugt worden ist (z.B. alle Formulare, die automatisch erzeugt werden), dann kannst du über die Windows API herausfinden, ob du dich im VCL Hauptthread befindest:

if( ::GetWindowThreadProcessId( Form->Handle ) == ::GetCurrentThreadId() )
{
   // Thread IDs stimmen überein => wir sind im VCL Hauptthread
}
else
{
   // Thread IDs stimmen nicht überein => wir sind nicht im VCL Hauptthread
}

CppDude

Danke für die Aufklärung wegen dem OnClick.

Ihr meint das wirklich Ernst mit dem "kein Thread notwendig" ??
Die Anwendung würde doch mega "hängen bleiben" wenn ich da 36 TPanel erstelle
und dafür keinen Thread verwenden würde.

Nebenbei läuft ja auch noch Sound + Lebensanzeige.
Wie soll das denn mit nem Timer flüssig laufen.

Edit: ... werde das mal Testen mit den IDs, danke.

DocShoe

Naja, kommt drauf an. Du kannst bei Programmstart alle Panels erzeugen und dann nur das jeweils benötigte anzeigen. Dann hast du beim Programmstart eine kurze Verzögerung, weil die Panels erzeugt werden müssen, ersparst dir aber die ganze Thread Hampelei. Ein normaler Windows Timer löst mit etwa 15ms auf, das sollte für deine Belange mehr als ausreichend sein.

CppDude

Hm... werd ich mir heute mal anschauen. Zumindest werde ich mir ein neues Projekt erstellen
und da mal versuchen das Thema mit dem Timer zu lösen und auf die Threads zu verzichten.

Ich glaube noch nicht so wirklich daran, dass sich das "flüssig" abspielen lässt, wenn ich
dann auch noch den Sound nebenbei abspiele und er ständig Berechnungen für Punktzahl und
Lebenspunkte durchführt.

Aber ich werde es trotzdem mal versuchen um zu sehen ob die Threads wirklich so uberflüssig
sind für diesen Anwendungsbereich.

Schonmal danke für die ganzen Erklärungen und die Hilfe.
Das bring tmich auch für zukünftige Projekte weiter.

Th69

Hallo nochmals,

durch das Synchronize() hast du ja bisher auch die ganzen Panels im VCL-Hauptthread erzeugt (anders geht es ja auch nicht), d.h. die Umstellung auf den Timer sollte also keine Performancebremse sein

Ich habe mir jetzt noch mal im Detail deinen Thread-Execute() angeschaut:

while (!Terminated)
    {
      // Zeitberechnung
      // ...

      // aktuelle Laufzeit anzeigen
      // (wird aller 1000 ms aufgerufen)
      if ( ::GetTickCount() - TimeStart >= 1000)
      {
        Synchronize(ShowZeit);

        // Startzeitpunkt reset
        TimeStart = ::GetTickCount();
      }

      // Pause um Prozessorauslastung nicht auf 100% zu treiben
      Sleep(Pause);
    }

Du hast damit also nur einen Timer simuliert (und das auch noch äußerst ungünstig, da bis auf die Zeitabfrage eh alles im VCL-Hauptthread abläuft, d.h. der Threadwechsel ist hier teurer als dein eigentlicher Code)!

Fazit: Threads nur dann verwenden, wenn wirklich Background-Aktionen (z.B. längere Berechnungen, Simulationen o.ä) ausgeführt werden sollen. Interaktionen (Animationen, Ein-/Ausblendeffekte etc.) mit dem GUI immer über einen Timer realisieren.