Erstellung eines Worker-Threads zur Grafikausgabe

elmut19

@yahendrik
jetzt sind wir bei den Dingen angelangt, wo es ans Eingemachte geht
und die ich auch noch nicht verstanden habe.
Aber das ist ja auch das wichtige an de Sache, um eine vernünftige Software hinzubekommen.
Und vermutlich muss ich das auch so nach und nach dahingehend optimieren.

Meine "Kill"-Funktion:

void CViewGrafik::CleanUpGrafikThread()
{
	TerminateThread(m_hGrafikThread, -1);
	CloseHandle(m_hGrafikThread);
	m_hGrafikThread = 0;
}

Nun habe ich auch teilweise seltsames Verhalten festgestellt:

hatte ich Abstürze, weil meine "DrawGrafik()" kurz nach dem Aufruf über den Button (und bevor der "GrafikThread()" gestartet war), ein weiteres mal ausgeführt wurde.
Dadurch wurde "InitPen()" nochmals aufgerufen. Das konnte ich durch ein "Sleep(1000)" am Ende von "DrawGrafk()" seltsamerweise beheben.
Bin ich auch noch am Testen.

DocShoe

@elmut19
TerminateThread birgt so einige Gefahren in sich, das solltest du anders lösen. Du könntest eine Klasse um den Thread bauen, damit du eine Abbruchbedingung implementieren kannst:

class ScopedLock 
{
   CRITICAL_SECTION& CriticalSection_ ;
public:
   ScopedLock( CRITICAL_SECTION cs ) :
      CriticalSection( cs )
   {
      // to do: Fehlerbehandlung
      EnterCriticalSection( &CriticalSection_ );    
   }

   ~ScopedLock ()
   {
      // to do: Fehlerbehandlung
      LeaveCriticalSection( &CriticalSection_ );    
   }
   // Objekt darf nicht kopierbar sein
   ScopedLock( ScopedLock const& ) = delete;
   ScopedLock( ScopedLock&& ) = delete;
   ScopedLock& operator=( ScopedLock const& ) = delete;
   ScopedLock& operator=( ScopedLock&& ) = delete;
};

class Win32Thread
{
   CRITICAL_SECTION CriticalSection_;
   HANDLE Thread_ = 0;
   bool Terminated_ = false;
public:
   Win32Thread()
   {
      // to do: Fehlerbehandlung
     CreateCriticalSection( &CriticalSection );
   }

   ~WinThread()
   {
      DestroyCriticalSection( &CriticalSection );
   }

   void terminate()
   {
      ScopedLock lock( CriticalSection_ );
      Terminated_ true;
   }

   bool terminated() const
   {
      ScopedLock lock( CriticalSection_ );
      return Terminated_;
   }

   void join()
   {
      // auf Threadende warten
      WaitForSingleObject( Thread_, INFINITE );
   }

   void run()
   {
      while( !terminated() )
      {
         if( !terminated() ) draw_step_1();
         if( !terminated() ) draw_step_2();
         if( !terminated() ) draw_step_...();
         if( !terminated() ) draw_step_n();      
      }
   }
}

So könnte ein Gerüst für ein Win32 Thread aussehen. Du kannst das auch mit std::thread lösen, nur kenne ich mich mit denen kaum aus. Die Idee ist, dass dein Thread so lange läuft, bis du ihm per terminate() signalisierst, dass er sich zum nächstmöglichen Zeitpunkt beenden soll (=> die run() Methode verlässt). Mit join() wartest du darauf, dass er das tut.
In der run() Methode solltest du in kürzeren Abständen prüfen, ob der Thread beendet werden muss, damit der Thread nicht zu träge auf die Abbruchaufforderung reagiert.
Der Übersicht halber ist das Ganze ohne Netz und doppelten Boden.

elmut19

@DocShoe
Vielen Dank DocShoe.
Bin nun zurück aus dem Wochenende.
Nun muss ich mir das Ganze durch den Kopf gehen lassen.

Also, mein Thread beendet sich selbst ja auch mit "ExitThread(0)", wenn er denn so weit ist.
Also von dem "TerminateThread()" mal abgesehen.

Frage zur "CriticalSection":
Diese sorgt hier wohl nur dafür, dass der Thread nicht ein zweites Mal gestartet werden kann,
bevor der gerade laufende Thread nicht komplett beendet ist?
Er schützt sich dann also nur vor sich selbst?
Das macht sicher Sinn!

Eine zusätzliche Abfrage in der Ausgabeschleife, ob der Thread von Aussen beendet werden soll, macht sicher auch Sinn,
da ich ja sonst nicht wissen kann, wann er dann wirklich beendet ist.
Obwohl ich so vom Verhalten der Software haptisch noch keine Probleme bemerkt habe.
Aber ausschliessen kann ich es mit meiner Variante wohl nicht.

Also vielen Dank noch dafür.
Ich werde mich rantasten.

DocShoe

@elmut19
Nein, eine CriticalSection verriegelt die Ausführung von bestimmten Codeabschnitten für mehrere Threads. Eine CriticalSection kann nur von einem Thread gleichzeitig betreten werden, und solange ein Thread eine CriticalSection betreten hat kann kein anderer Thread diesen Codeabschnitt betreten. Er wartet dann so lange, bis die CriticalSection wieder verlassen wird. Damit wird verhindert, dass ein Thread einen Wert liest, während ein anderer ihn gerade schreibt.
C++ unterstützt sowas auch mit std::atomic ab C++11, ich weiß aber nicht, wie das intern synchronisiert wird. CriticalSections sind unter Windows wohl das Schnellste.

Was die Haptik deiner Software angeht:
Starte doch mal 10.000 oder 100.000 deiner Threads und beende sie mit TerminateThread. Mach dir zu Beginn des Tests einen Snapshot über die Statistik deiner Anwendung (benötigter Speicher, Handles) und vergleich´ das nach Testende. Wenn dir da iwas wegläuft hast du ein Problem, das früher oder später garantiert beim Kunden auftritt

elmut19

Nein, eine CriticalSection verriegelt die Ausführung von bestimmten Codeabschnitten für mehrere Threads.

Ich benutze schon einige CriticalSections in der Software.
Momentan fällt mir aber kein anderer Thread ein, wo ich an dieser Stelle noch einen zusätzlichen Schutz einbauen müsste.

DocShoe

@elmut19

Ähm... du hast einen Thread, der die Grafikausgabe macht und den Hauptthread deiner Anwendung. Beide greifen auf gemeinsame Daten zu (im Realfall wohl die anzuzeigenden Daten, in meinem Beispiel nur das Terminate-Flag). Ein Thread schreibt, ein anderer liest. Ohne Synchronisierung führt das iwann zu lustigen Ergebnissen. Das Debuggen ist dann meist weniger lustig. Da brauchste bei Race Conditions einiges an Glück, um den Fall zu finden, wo´s in die Hose geht.

elmut19

@DocShoe
Ja. Das hoffe ich durch die bestehenden Kriterien schon ausgeschlossen zu haben.
Aber im Einzelfall muss ich das wohl, wegen geänderter Bedingungen nochmals prüfen.
Nicht dass ich durch einen Haufen CriticalSections noch einen DeadLock produziere.

elmut19

@DocShoe
Nun hoffe ich, nach langem Suchen, dass ich eine für dieses Problem gangbare Variante gefunden habe.
Den Thread starte ich nun über "_beginthreadex()" und "_endthreadex().
Zum externen beenden verwende ich noch "InterlockedCompareExchange()" und "InterlockedExchange()".
Diese beiden brechen den Thread jeweils an geeigneten Stellen ab.
Auch habe ich überprüft, ob durch ein "TerminateThread()" etwas zurückbleiben kann, da ich nicht
sehr lange auf das "normale" Ende des Thread warten kann.
Eine kurze Zeitspanne kann ich über setzen eines Event am Ende des Threads warten.
Da ich ausschliesslich mit Membern meiner "CViewGrafik" arbeite, die anschliessend eh behandelt werden,
kann hier nichts passieren.
Auch kann in dieser Programmebene kein anderer Thread auf interne Daten zugreifen.
So war es vorher auch schon (Zustands-Flags und Crit.Sect.).

Auf jeden Fall danke noch für die Unterstützung.

DocShoe

Dann lies dir mal den Remarks Abschnitt durch.

hustbaer

@elmut19 sagte in Erstellung eines Worker-Threads zur Grafikausgabe:

Da ich ausschliesslich mit Membern meiner "CViewGrafik" arbeite, die anschliessend eh behandelt werden,
kann hier nichts passieren.

Falsch.

elmut19

@DocShoe sagte in Erstellung eines Worker-Threads zur Grafikausgabe:

Dann lies dir mal den Remarks Abschnitt durch

Die Remarks habe ich gelesen, auch vorher schon.
Ich sehe ein, dass es eine heikle Sache ist.
Die 4 Punkte konnte ich allerdings positiv beantworten.

Jedenfalls muss ich sicherstellen, dass ich den Thread loswerde!
Und das muss auch ohne Beeinträchtigung des Anwenders erfolgen.

@hustsbear
Aber was ist noch falsch?
Es ist auch sichergestellt, dass das Thread-Handle geschlossen wird.
Ich brauche eine Möglichkeit, den Thread nach einer kurzen Zeit zu 100% loszuwerden.
Ein "WaitFor..(... INFINITE)" ist keine Lösung!
Ein WaitFor..(... ZWEI_SEKUNDEN) wäre angemessen.
Das kann ich über den Event am Thread-Ende gewährleisten.
Und das ist auch drin.
Und über das Flag, das ich an den Thread sende, wird dieser auch beschleunigt abgebrochen.

Aber irgendeine Sicherheit muss ich noch behalten.

hustbaer

@elmut19 sagte in Erstellung eines Worker-Threads zur Grafikausgabe:

@hustsbear
Aber was ist noch falsch?

Na der Satz den ich zitiert habe:

Da ich ausschliesslich mit Membern meiner "CViewGrafik" arbeite, die anschliessend eh behandelt werden,
kann hier nichts passieren.

Du rufst da drin ja Win32 und MFC Funktionen auf. Diese Funktionen ... machen Dinge. Und wenn sie z.B....

diese Dinge zumindest teilweise im Usermode machen und
dazu Locks (Critical Sections, SRWLocks, ...) verwenden

dann kann dir das übel um die Ohren fliegen. (Nur damit das klar ist: Das ist nicht die einzige Möglichkeit wie es zu einem Problem kommen kann, es ist nur ein Beispiel.)

Wenn du einen Thread mit TerminateThread abschiesst, dann kannst du nicht kontrollieren wo er abgebrochen wird. D.h. er könnte z.B. eine Critical Section gelockt haben. Die bleibt dann gelockt und dein Programm wird halt einfach hängen bleiben sobald die nächste Funktion aufgerufen wird die die selbe Critical Section locken muss.

Also ist die Schlussfolgerund es "kann hier nichts passieren" falsch. Es kann sehr wohl was passieren.

Ich brauche eine Möglichkeit, den Thread nach einer kurzen Zeit zu 100% loszuwerden.
Ein "WaitFor..(... INFINITE)" ist keine Lösung!

Und genau das ist auch wieder falsch. Ein WaitFor..(... INFINITE) ist genau die Lösung. Du musst halt sicherstellen dass der Mechanismus den du verwendest um den Thread dazu zu bringen sich selbst zu beenden zuverlässig ist.

Ein WaitFor..(... ZWEI_SEKUNDEN) wäre angemessen.

Nein! Ich verstehe ja deine Sorge, vor vielen Jahren als ich selbst das erste mal mit diesem Problem konfrontiert war hab ich mir das selbe gedacht. Und die Versuchung ist gross hier ein "Not-Aus" einbauen zu wollen. Nur glaub mir bitte: es ist besser es nicht zu tun. Wenn sich der Thread nicht von selbst beendet, dann hat dein Programm einen Fehler. Den Thread nach ein paar Sekunden abzubrechen macht aber alles nur noch schlimmer. Es maskiert den Fehler, mit einer "Lösung" die die unangenehme Eigenschaft hat selbst wieder Probleme erzeugen zu können. Und noch dazu Probleme die man dann kaum festnageln kann.

Was du machen kannst wenn du wirklich meinst es sei notwendig: Wenn der Thread sich nach ein paar Sekunden immer noch nicht selbst beendet hat, dann beende einfach den ganzen Prozess. Idealerweise schreibst du vorher noch einen Crash-Dump damit du nachher was hast womit du schauen kannst warum der Thread sich nicht beendet hat: https://docs.microsoft.com/en-us/windows/win32/api/minidumpapiset/nf-minidumpapiset-minidumpwritedump

DocShoe

Weiter oben habe ich ja schon vorgeschlagen, die Abbruchbedingung in der Threadfunktion häufiger zu prüfen, und nicht nur bei jedem Schleifendurchlauf. Die häufige Überprüfung kostet zwar etwas Performance, weil jedes Mal eine CriticalSection betreten werden muss, aber damit reagiert der Thread zügiger auf die Abbruchbedingung. Wenn der Threadabbruch zu träger wird musst du halt die Intervalle zur Überprüfung der Abbruchbedingung verkürzen.

hustbaer

@DocShoe sagte in Erstellung eines Worker-Threads zur Grafikausgabe:

Die häufige Überprüfung kostet zwar etwas Performance, weil jedes Mal eine CriticalSection betreten werden muss, aber damit reagiert der Thread zügiger auf die Abbruchbedingung.

Wozu eine CriticalSection?
std::atomic<bool>::load(std::memory_order_acquire) ist völlig ausreichend. Und das ist auf x86 ein einfacher, billiger Speicherzugriff.
(Meistens wird sogar std::memory_order_relaxed reichen, aber man muss ja nicht übertreiben.)

DocShoe

@hustbaer

Ich kenn mich mit dem C++11 Synchronisierungskram nicht aus, unsere Software ist älter und es gibt eigene Wrapper für die Win32 Objekte. Damit sind wir bisher gut gefahren und sehen atm keinen Grund zur Umstellung.
Und ja, ich müsste mich mal mit den C++11 Neuerungen beschäftigen. Mehr gibt unser Compiler leider nicht her.

elmut19

@DocShoe
Ich arbeite zwar für mich selbst inzwischen mit VS2019.
Aber für unser Kunden wird immer noch die exe mit VS2005 erstellt.
Es ist einfach keine Zeit, die ganze Installation umzustellen und wieder neue Libs mit zu verteilen.

DocShoe

@elmut19
Auf was genau beziehst du dich?

hustbaer

@DocShoe Vermutlich darauf dass Visual Studio 2005 kein C++11 kann

@elmut19 Du kannst auch Boost.Atomic verwenden.

Bzw. wenn's dich nicht stört dass es nicht zu 100% vom C++ Standard abgedeckt ist sondern nur von einer zusätzlichen Garantie von Visual Studio, dann kannst du für sowas wie ein Cancel-Flag auch einfach ein volatile bool verwenden. VS 2005 garantiert bei volatile Variablen mit passendem Alignment dass:

Lese und Schreibzugriffe immer atomar sind
Lesezugriffe immer "acquire" Semantik haben
Schreibzugriffe immer "release" Semantik haben
(Einschränkung: die Garantie dass Zugriffe atomar sind steht dort nicht explizit und gilt auch nur für bestimmte Typen. Also z.B. nicht für structs.)

In neueren VS Versionen ist diese Garantie abhängig vom /volatile Switch: https://docs.microsoft.com/en-us/cpp/build/reference/volatile-volatile-keyword-interpretation?view=msvc-160

elmut19

@DocShoe
Ja, beim Sprachstandard bin ich einfach noch etwas hinterher.
Und nun noch BOOST einzubauen, schient mir auch nicht so sinnvoll.
Es gibt auch so noch viel Potential für ein Reengineering.
Zumindest eine der Grafiken habe ich auch durch weglassen mehrfacher identischer Punkte um ca. 50% schneller bekommen