3. std::thread abbrechen mit atomic_bool

std::thread abbrechen mit atomic_bool

  • H

    TyRoXx schrieb:

    Es gibt dafür eine Lösung: Nimm einen echten Kindprozess im Sinne von fork . Den kann man gefahrlos töten.

    "Im Sinne von fork " unter Windows wäre dann vermutlich _beginthreadex , richtig?

    Wenn man den dann abschießt läuft man auch nicht Gefahr irgendwelche Resourcen zu leaken? Bzw. warum kann man den standard std::thread nicht beenden (dachte das liegt an der Schwierigkeit wieder "aufzuräumen"), oder sehe ich das falsch? Und wenn std::thread kein Kindprozess des aufrufenden Prozesses ist, warum gibt es dann sowas wie std::child_process nicht?

    0
  • T

    happystudent schrieb:

    Und zwar greift der thread den ich starte auf ein Textfeld zu, in dem ein string gespeichert ist.

    Da ist dein Fehler. Normalerweise gibt man dem Hintergrund-Thread eine Möglichkeit den GUI-Thread zu bitten etwas am GUI zu machen. So pauschal kann ich dazu aber nicht viel sagen, das hängt von der Anwendung ab.

    happystudent schrieb:

    TyRoXx schrieb:

    Es gibt dafür eine Lösung: Nimm einen echten Kindprozess im Sinne von fork . Den kann man gefahrlos töten.

    "Im Sinne von fork " unter Windows wäre dann vermutlich _beginthreadex , richtig?

    Ne, eher CreateProcess .

    happystudent schrieb:

    Wenn man den dann abschießt läuft man auch nicht Gefahr irgendwelche Resourcen zu leaken? Bzw. warum kann man den standard std::thread nicht beenden (dachte das liegt an der Schwierigkeit wieder "aufzuräumen"), oder sehe ich das falsch?

    Man kann den unter anderem deswegen nicht abschießen, weil das unnötigen Overhead bedeuten würde, selbst, wenn man das Abschießen gar nicht nutzt.
    Beispiel: Wenn der Thread gerade ein mutex gelockt hat, müsste das Abschießen entweder warten, bis der mutex freigegeben wird oder den selber nach dem Abschießen freigeben. Beide Varianten würden mutex und ähnliche Dinge für jeden teurer machen und die Implementierung der Standardbibliothek noch schwieriger.
    Man könnte natürlich so etwas wie interruptible_mutex einbauen, aber das hat einfach noch keiner standardisiert.

    happystudent schrieb:

    Und wenn std::thread kein Kindprozess des aufrufenden Prozesses ist, warum gibt es dann sowas wie std::child_process nicht?

    Weil es noch keiner standardisiert hat. Für Prozessverwaltung gibt es einfach noch keinen Konsens. Nicht einmal Boost hat etwas dafür.

    0
  • T

    happystudent schrieb:

    Hm Ok... Dann werd ich es wohl so machen müssen. Danke auf jeden Fall 👍

    EDIT:

    Also ich hab noch ein paar Fragen... Und zwar greift der thread den ich starte auf ein Textfeld zu, in dem ein string gespeichert ist. Wenn jetzt ein keyboard_event auftritt, kann es sein dass das Textfeld danach (zum Beispiel) komplett leer ist (alles gelöscht). Der (noch laufende) thread würde ja jetzt versuchen an der Stelle n (die vor dem Löschvorgang noch existiert hat) des strings zuzugreifen und damit crashen.

    Die einzige Möglichkeit die ich momentan sehe um das zu vermeiden, ist vor jedem Lesevorgang zu checken ob meine atomic_bool variable sich verändert hat. Da die Funktion sehr viele solche Zugriffe durchführt jetzt die Frage: Ist das nicht sehr teuer? Oder ist ein Lesezugriff auf ein atomic_bool von der Geschwindigkeit her ähnlich wie ein "normales" bool (und damit vernachlässigbar)?

    Wie würdet ihr sowas lösen?

    Erst mal solltest Du prüfen, ob Du überhaupt auf das Textfeld aus einem anderen Thread zugreifen darfst. Das geht in der Regel nicht. Die Methoden dafür sind in der Regel nicht threadsafe. Das hängt von der verwendeten GUI ab.

    Ich verstehe nicht ganz, was Du mit dem atomic bool erreichen willst aber es hört sich nicht gut an. Du musst bedenken, dass nach der Abfrage des flags das Textfeld gelöscht werden kann bevor Du darauf zugreifen kannst.

    0
  • H

    TyRoXx schrieb:

    Da ist dein Fehler. Normalerweise gibt man dem Hintergrund-Thread eine Möglichkeit den GUI-Thread zu bitten etwas am GUI zu machen. So pauschal kann ich dazu aber nicht viel sagen, das hängt von der Anwendung ab.

    Ok, kein Problem, das Design kann ich soweit noch ändern. Ich suche momentan nach einem guten Konzept um das mit den threads zu bewerkstelligen.

    tntnet schrieb:

    Erst mal solltest Du prüfen, ob Du überhaupt auf das Textfeld aus einem anderen Thread zugreifen darfst.

    Ja nee, darf ich nicht. Fällt mir gerade nach Lesen der Doku auf, das ist schlecht 😃

    tntnet schrieb:

    Ich verstehe nicht ganz, was Du mit dem atomic bool erreichen willst aber es hört sich nicht gut an. Du musst bedenken, dass nach der Abfrage des flags das Textfeld gelöscht werden kann bevor Du darauf zugreifen kannst.

    Ja, das hab ich befürchtet... also mit dem atomic bool werd ich nicht weiterkommen merk ich schon.

    Wie mache ich das dann?

    Ich könnte natürlich den gesamten Text des Textfeldes erstmal in einen string holen und diesen dann dem thread mitgeben. Aber das ist bei viel Text sehr teuer und vor allem unnötig, weil der child-thread nur einen Bruchteil des ganzen Textes braucht (welchen Teil, muss er aber selbst bestimmen).

    0
  • T

    happystudent schrieb:

    Ich könnte natürlich den gesamten Text des Textfeldes erstmal in einen string holen und diesen dann dem thread mitgeben. Aber das ist bei viel Text sehr teuer und vor allem unnötig, weil der child-thread nur einen Bruchteil des ganzen Textes braucht (welchen Teil, muss er aber selbst bestimmen).

    Ist das so? Hast du es ausprobiert?
    Wenn das GUI keine Möglichkeit anbietet den Text zwischen Threads zu teilen, dann bleibt nur die Kopie.

    0
  • H

    TyRoXx schrieb:

    Ist das so? Hast du es ausprobiert?
    Wenn das GUI keine Möglichkeit anbietet den Text zwischen Threads zu teilen, dann bleibt nur die Kopie.

    Also die GUI (Scintilla) bietet dafür nur SendMessage an und das kann man ja immer und für alle Textfelder benutzen von daher weiß ich jetzt nicht ob du das auch gemeint hast ...

    Probiert hab ich das mit den threads noch nicht, ich bin wegen dem thread-abbrechen Problem noch nicht wirklich vorran gekommen seit ich das parallelisieren wollte ...

    0
  • F

    happystudent schrieb:

    Ich könnte natürlich den gesamten Text des Textfeldes erstmal in einen string holen und diesen dann dem thread mitgeben. Aber das ist bei viel Text sehr teuer und vor allem unnötig, weil der child-thread nur einen Bruchteil des ganzen Textes braucht (welchen Teil, muss er aber selbst bestimmen).

    Bist du dir absolut sicher dass das ein Performance-Problem darstellt? Den Text dem Thread "mitzugeben" hört sich so an, als würdest du den String beim erstellen des Threads mit übergeben wollen. Der Text müsste schon ziemlich lang sein, damit die Kosten der zusätzlichen Kopie im Vergleich zu den Kosten der Threaderstellung überhaupt spürbar wären.

    Ohne zumindest ein wenig Pseudocode zu sehen, klingen deine ganzen Performance-Bedenken hier extrem verfrüht.

    Ich würde dir raten erst einmal einfachen Code ohne allzu viel Schnickschnack zu schreiben und erst dann verschiendene Implementationen zum vergleich heranziehen (sei es weil sich ein Performance-Problem manifestiert, oder aus Neugier, welche Variante die schnellste ist).

    Als Anregung kann ich dir vielleicht noch mitgeben wie ich z.B. Probleme dieser Art lösen würde:

    Ich gehe davon aus, das du eine solche oder ähnliche Datenstruktur hast:

    struct Data
{
    std::string text;
};

Data sharedData;

    wobei mehrere Threads auf sharedData zugreifen können (sharedData sei hier nur beispielhaft, das kann z.B. auch das Textfeld deiner GUI sein).
    Wenn alle Threads nur lesend auf sharedData zugreifen, brauchst du gar keine Synchronisation, und du kannst schamlos damit hantieren (hier sei angenommen, dass std::cout thread-sicher ist, das ist nicht immer der Fall):

    Thread 1:
...
std::cout << sharedData.text.size() << std::endl;
...

Thread 2:
...
std::cout << sharedData.text << std::endl;
...

    Wenn mindestens ein Thread schreibend auf sharedData zugreift, und das Objekt nicht für diesen Fall thread-sicher implementiert wurde, würde ich den Zugriff zuerst einmal durch einen Mutex schützen:

    struct Data
{
    // Anmerkung: "mutable" ermöglicht es, den Mutex auch dann
    // zu "locken" wenn man über ein "const Data" darauf zugreift.
    // Da es sich bei dem Mutex lediglich um ein internes 
    // Synchronisationsobjekt für die eigentlichen Daten handelt
    // ist ein Mutex einer der wenigen Anwendungsfälle, bei denen
    // das sich das "mutable"-Schlüsselwort nicht gleich eines
    // schlechten Programmierstils verdächtig macht ;-)
    mutable std::mutex mutex;
    std::string text;
};

Data sharedData;

Thread 1:
...
{
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    sharedData.text = neuerText;
}
...

Thread 2:
...
{
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    teureBerechnungMitText(sharedData.text);
}
...

    Das sollte für die meisten Anwendungsfälle ausreichend sein.

    Wenn sich herausstellt, dass sich die Threads dabei zu sehr gegenseitig auf den Füssen stehen (es kann immer nur ein Thread ein "Lock" auf den Mutex halten), weil sie z.B. zu lange mit dem sharedData /string beschäftigt sind ( teureBerechnungMitText ), und daher das "Lock" zu lange halten, kann man z.B. überlegen, ob man den Text nicht vorher lokal kopiert (thread-lokale Daten müssen auch nicht synchronisiert werden):

    Thread 2:
...

std::string lokalerText;

{
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    lokalerText = sharedData.text;
}

std::cout << lokalerText << std::endl;
teureBerechnungMitText(lokalerText);
...

    Oder wenn das Kopieren des Textes auch noch zu teuer erscheint, kann man vielleicht die Tatsache nutzen, dass Zuweisungen zwischen std::shared_ptr thread-sicher sind (intern meist durch atomare Operationen implementiert) und ein "konstantes" Objekt per se als thread-sicher angesehen werden kann, da man nur lesend darauf zugreifen kann:

    struct Data
{
    std::shared_ptr<const std::string> text;
};

Data sharedData;

Thread 1:
...
auto neuerString = std::make_shared<const std::string>(neuerText);
sharedData.text = neuerString;
...

Thread 2:
...
auto lokalerText = sharedData.text;
teureBerechnungMitText(*lokalerText);
...

    Diese Idee hat gewisse Ähnlichkeiten mit COW-Strings (copy-on-write), und macht natürlich in dieser Form nur Sinn, wenn teureBerechnungMitText den Text nur liest und nicht verändert (ansonsten muss man auf den Mutex oder die lokale Kopie zurückgreifen).

    Gruss,
    Finnegan

    0
  • ?

    Mutexe sind oft ein Zeichen schlechter Parallelisierung.

    Anstatt

    void f() { // wird parallel ausgeführt
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    lokalerText = sharedData.text;
    ...
}

    lieber

    void f(std::string lokalerText) { // wird parallel ausgeführt
    ...
}

    und anstatt

    void g() { // wird parallel ausgeführt
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    sharedData.text = neuerText;
}

    lieber

    std::string g() { // wird parallel ausgeführt
    return neuerText;
}
// Der Aufrufer kümmert sich um die Synchronisation (irgendein Workaround um future.then())

    Mit Progressbar und Abbrechen sieht dass so aus:

    struct calculation_state {
  std::atomic_bool running;
  std::atomic_int progress;
}
std::pair<bool, std::string> h(std::string lokalerText, calculation_state& state) {
  for (...) { // some loop
    if (!running) return {false, {}}; // failure
    ...
    state.progress++;
  }
  return {true, std::move(neuerText)};
}

    Selbst Berechnungs-Updates können versteckt werden:

    struct calculation_state {
  std::atomic_bool running;
  std::atomic_int progress;
  std::atomic<std::string*> partial_result; // only valid whenn running=true!
}
std::pair<bool, std::string> h(std::string lokalerText, calculation_state& state) {
  std::string partial_result[2];
  for (...) { // some loop
    if (!running) return {false, {}}; // failure
    ...

    partial_result[state.progress%2] = lokalerText; // kopiert den aktuellen Stand
    state.partial_result = &partial_result[state.progress%2]; // darf nicht mehr verändert werden, solange state.partial_result darauf zeigt!
    state.progress++;
  }
  state.running = false;
  return {true, std::move(neuerText)};
}
    0
  • H

    Hallo,

    danke schonmal für die Antworten. Das ist viel neuer Input, werd ich jetzt mal in Ruhe durchgehen und danach hoffentlich schlauer sein 🙂

    Danke auf jeden Fall schonmal 👍

    0
  • F

    lockvogel schrieb:

    Mutexe sind oft ein Zeichen schlechter Parallelisierung.

    Anstatt

    void f() { // wird parallel ausgeführt
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    lokalerText = sharedData.text;
    ...
}

    lieber

    void f(std::string lokalerText) { // wird parallel ausgeführt
    ...
}

    und anstatt

    void g() { // wird parallel ausgeführt
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    sharedData.text = neuerText;
}

    lieber

    std::string g() { // wird parallel ausgeführt
    return neuerText;
}
// Der Aufrufer kümmert sich um die Synchronisation (irgendein Workaround um future.then())

    So wie du das dort schreibst macht das durchaus Sinn, allerdings war es nicht meine Absicht, mit den Beispielen die Threadfunktionen so genau auszuformulieren. Das habe ich mit den "..." bewusst außen vor gelassen, und wollte die Beispiele damit möglichst agnostisch gegenüber dem Kontext formulieren, in dem der Code ausgeführt wird. Wenn ich also schreibe

    Thread 2:
...
{
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    teureBerechnungMitText(sharedData.text);
}
...

    Dann ist damit nicht zwangsläufig eine Threadfunktion gemeint, die beendet, sobald teureBerechnungMitText fertig ist, sondern beliebiger Code, der lesend auf sharedData zugreifen muss, während gleichzeitig "mindestens ein Thread schreibend auf sharedData zugreift". Auch könnten die obigen Zeilen in einer Schleife stehen, in der die Berechnung mehrfach für den jeweils aktuellen Text durchgeführt wird.

    Für deine Funktionen könnte der "Thread 2" z.B. auch der Main-Thread sein, der einen Thread für deine obige Funktion f() erzeugen muss, während ein anderer Thread den Text verändern kann:

    Main Thread:
...
std::thread t;
{
    std::lock_guard<std::mutex> lock(sharedData.mutex);
    t = std::thread(f, sharedData.text);
}
...

    Dein Vorschlag ist gut, wenn der Thread nur einmal eine Berechnung auf einem gegebenen Text durchführen muss, und sich dann beendet. Allerdings kann man so trotzdem nicht auf einen Mutex oder eine andere Art der Synchronisation verzichten, wenn ein schreibender Thread jederzeit dazwischenfunken kann. Ob das jetzt beim Erstellen des Threads oder im Thread selbst jeweils einmal passiert, ist in beiden Fällen gleich schädlich für die Parallelisierung.

    Finnegan

    0
  • H

    Hallo nochmal,

    hab mir jetzt mal was gebastelt was soweit schon funktioniert. Den Text der GUI hab ich mal außen vor gelassen, da man den anscheinend thread-safe bekommen kann. Ich hab mir gedacht dass ich einfach zu begin jeder Unter-Funktion die atomic_bool variable checken könnte und falls diese sich geändert hat, einfach eine eigene Exception zu werfen. Damit müsste ich die Information auch nicht quer durch etliche Unterfunktionen weiterreichen:

    #include <iostream>
#include <chrono>
#include <thread>
#include <atomic>

class background_worker
{
	struct thread_abort_exception : std::exception {};
	static std::atomic<bool> running;
public:
	void stop_work() {
		running = false;
	}

	void expensive_subroutine() {
		if (!running) {
			throw thread_abort_exception();
		}
		std::this_thread::sleep_for(std::chrono::milliseconds(1));
	}

	void operator()() {
		try {
			for (size_t i = 0; i < 2000; ++i) {
				expensive_subroutine();
				std::cout << i << '\n';
			}
			std::cout << "expensive stuff done!\n";
		}
		catch (thread_abort_exception const &) {
			std::cerr << "thread aborted!\n";
		}
	}

	void start_work() {
		std::thread t(*this);
		t.detach();
	}
};

std::atomic<bool> background_worker::running{ true };

int main() {
	background_worker bw;
	bw.start_work();

	while (true) {
		int input;
		std::cin >> input;

		if (input) {
			bw.stop_work();
		}
	}
}

    Dazu habe ich jetzt mehrere Fragen:

    • Spricht etwas gegen diesen Ansatz oder kann man das so machen?
    • Ist es "teuer" eine atomic_bool Variable so oft zu lesen oder ist das normalerweise vernachlässigbar?
    • Meine atomic_bool Variable muss momentan static sein, wenn ich diese non-static mache erhalte ich Fehlermeldungen dass ich den deleteten Copy-Konstruktor verwenden will - Definiere ich mir diesen selber kompiliert das Program zwar, aber der Code bekommt eine Änderung der Variable nicht mehr mit (also der thread wird nicht abgebrochen) - woran liegt das bzw. wie könnte man das lösen?
    • Die Schleife sollta ja immer 2 Sekunden laufen (2000 * 1ms sleep). Das tut sie auch "oft", allerdings manchmal auch nicht, dann läuft sie extrem langsam (Faktor 10) - auch das kann ich nicht nachvollziehen?

    Die oben angesprochenen Punkte lassen mich vermuten dass der Code sub-optimal ist. Würde mich daher über Verbesserungsvorschläge freuen.

    0
Anmelden zum Antworten