Warum stopt der Thread nicht?

Hi

TobiBob schrieb:

Ja, die Abbruchmethode wird aus dem GUI-Thread aufgerufen. Das heißt, ein Invoke von einem workthread auf die GUI blockiert diese? Hat das nichts mit der Statusvariablen zu tun? Auch wenn ich nur alle 20 ms neu frage? Hmm....

Der Gui-Thread hängt hier fest: while (!Interlocked.Equals(this.thrStatus, 0)){ Sleep }
Dann versucht der Worker mit Invoke in den Gui-Thread zu wechseln. Der hängt aber immer noch und zwar bis in alle Ewigkeit.
Klassischer Deadlock eben.

TobiBob schrieb:

Ich hab jetzt jedenfalls als Abbruchbedingung eine Abfrage auf den Threadstatus, statt die Statusvariable. Geht jetzt problemlos.

Zeig mal. Der Threadstatus sollte eigentlich nicht dafür verwendet werden.

Vielleicht solltest Du einfach auf die while-Schleife im Abbruchcode verzichten.
Siehe es als Signal an den Thread sich sobald wie möglich zu beenden aber lasse die Gui nicht darauf warten.
Dann könnte der Thread als letzte Aktion ein event feuern, an dem wiederum die GUI horcht. Damit weiß dieses auch wann der Thread tatsächlich seine Arbeit beendet hat.

TobiBob

µ schrieb:

(...)Dann versucht der Worker mit Invoke in den Gui-Thread zu wechseln. Der hängt aber immer noch und zwar bis in alle Ewigkeit.(...)

Ahhh! Damit geht mir ein großes Licht auf! Mir war nicht klar, dass mit Invoke() ein Threadwechsel stattfindet. Dann ist auch klar, warum der Compiler immer die Invoke-Funktionen etwas hervorgehoben hat.

Zur Warteschleife auf threadende:

while (this.threadOrbit.ThreadState == ThreadState.Running) ;

Ich habe aber gerade gemerkt, das wird irgendwie ignoriert... Warum sollte man das nicht benutzen?

Dein Vorschlag, den Thread einfach zuende laufen zu lassen und dann was zu feuern, hört sich natürlich sehr gut an. Werd ich machen.

hustbaer

Also es gibt diesbezüglich zwei Wege wie man es machen kann, die beide gut funktionieren.

a) Man schreibt die Worker-Thread Funktion so, dass sie nie auf den GUI Thread wartet (Heisst: nie darauf wartet dass der "Message Pump" des GUI Threads weiter läuft. Eine Mutex zu locken die im GUI-Thread immer nur kurz gehalten wird ist natürlich kein Problem). z.B. indem man BeginInvoke statt Invoke verwendet. Dann kann man im GUI Thread dem Worker-Thread ein Cancel-Flag setzen, und dann "synchron" darauf warten dass er terminiert. Dabei muss man dann nur wissen, dass ein mit "BeginInvoke" abgesetzter "Aufruf" im GUI-Thread "ankommen" könnte, nachdem der Worker-Thread bereits terminiert hat. Der GUI-Thread muss damit dann klarkommen ohne irgendwelchen Mist zu bauen.

b) Man schreibt die Worker-Thread Funktion so, dass sie auch direkt Invoke machen darf. Dann darf der GUI Thread aber nie nie nicht "synchron" darauf warten dass der Worker-Thread fertig wird. Eine Möglichkeit das zu "lösen" ist, indem man dem Worker-Thread das Cancel-Flag setzt, dann im GUI-Objekt vermerkt dass er gecancelt wurde, und den Worker-Thread danach einfach "vergisst". Das GUI-Objekt (z.B. Form) kann danach gerne "geschlossen" werden. Man muss hier nur auch damit rechnen, dass *danach* noch ein Invoke vom Worker-Thread ausgeführt wird. Und genau dazu hat man irgendwo vermerkt dass der Worker-Thread uns jetzt nicht mehr interessiert, und macht in der über Invoke aufgerufenen Funktion dann z.B. einfach gar nix.

Natürlich kann man a und b auch kombinieren, aber nur in der Form dass der Worker-Thread NIE Invoke macht, der GUI-Thread aber trotzdem nicht synchron auf den Worker-Thread wartet. Umgekehrt kombinieren (Invoke + synchron warten) geht wie du ja bemerkt hast leider nicht.

TobiBob schrieb:

Zur Warteschleife auf threadende:
while (this.threadOrbit.ThreadState == ThreadState.Running) ;
Ich habe aber gerade gemerkt, das wird irgendwie ignoriert... Warum sollte man das nicht benutzen?

Öhmja, da hast du hoffentlich ein Thread.Sleep() vergessen das in deinem "echten" Code vorhanden ist. Sonst würdest du da übel CPU-Zeit verbraten.
Und auf das Ende eines Threads zu warten, dafür gibt's ne eigene Funktion, die nennt sich Thread.Join.
http://msdn.microsoft.com/en-us/library/95hbf2ta.aspx
Das macht sinngemäss das selbe was du mit deiner Schleife machst, nur "besser" (z.B. ohne Polling)

TobiBob

@hustbaer, danke für die super Hinweise! Thread.Join hatte ich ganz vergessen, ist natürlich die bessere Lösung. Ich habe jetzt alle Invoke()-Aufrufe durch BeginInvoke() ersetzt. Es ist ja nur Text-in-drei-labels-schreiben, also ziemlich primitiv einfach - zu einfach, als dass ich Deine Hinweise wirklich voll darauf abtesten könnte, fürchte ich. Aber sie geben mir einen guten Eindruck von "good practice".

Was mich jetzt nur grad wundert: in der Dokumentation steht, jedes BeginInvoke() muss von einem EndInvoke() gefolgt sein. Das gilt in meinem Fall aber offenbar nicht.

Meine andere Überlegung jetzt gerade: da diese 3 Labels nur von dem Thread aus beschrieben werden: macht es da nicht Sinn, sie auch von dem Thread selber aus anzulegen? - Am Anfang mit Invoke einmal der Form hinzufügen, dann kann der Thread ständig direkt darauf zugreifen? ( Es werden einige Daten während einer Simulation in ziemlich schneller Abfolge in den Labels angezeigt.) Der Thread müsste dann nur immer aktiv bleiben, und nicht wie jetzt bei geänderten Startparametern neu gestartet werden. Macht sowas Sinn?

Hallo

TobiBob schrieb:

Ich habe jetzt alle Invoke()-Aufrufe durch BeginInvoke() ersetzt.

Das ist eine Lösung. Mit den Konsequenzen die hustbaer angesprochen hat.
Nämlich dass nach der Beendigung des Abbruchcodes und nach Threadende doch noch Aufträge in der GUI ausgeführt werden können.

Stell es Dir so vor:
Du hast einen Worker-Thread, der Invoke aufruft.
Das Invoke heißt soviel wie: "Hey Gui, bitte bearbeite mal den Code, den ich Dir hier übergebe, in Deinem Thread. Ich warte solange und blockiere den restlichen Ablauf meines Threads. Erst wenn Du, liebe GUI, fertig bist, werde ich weitermachen".

Ein BeginInvoke heißt: "Hey Gui: Ich habe hier Arbeit für Dich, die du bitteschön in Deinem verdammten Thread auszuführen hast. Aber ich habe null bock zu warten. Also kümmer Dich drumm. Irgendwann, sobald Du Zeit hast. Ist mir doch egal, ich mache jetzt mit meinem Zeug weiter." Und daraufhin führt der Worker seinen Threadcode aus ohne auf die Gui zu achten.

Also: Du schiebst mit BeginInvoke "Arbeit" zum GUI-Thread und beendest danach vielleicht den Worker-Thread. Dein Anfangsposting hörte sich aber so an, als wäre es Dir wichtig zu wissen, wann die ausgelagerte arbeit tatsächlich erledigt ist. Deshalb mein obiger Vorschlag mit dem Event bei Workerthread-Ende.

TobiBob schrieb:

in der Dokumentation steht, jedes BeginInvoke() muss von einem EndInvoke() gefolgt sein"

EndInvoke ist mit Thread.Join vergleichbar. Von "muss" kann keine Rede sein. Nur falls Du im aufrufenden Thread die Ergebnisse synchron erhalten willst, ist EndInvoke gefragt. Kleine Anmerkung: Mit den Methoden BeginInvoke und EndInvoke von Delegaten, kann jede Methode asynchron ausgeführt werden.

TobiBob schrieb:

macht es da nicht Sinn, sie auch von dem Thread selber aus anzulegen? - Am Anfang mit Invoke einmal der Form hinzufügen, dann kann der Thread ständig direkt darauf zugreifen?

Nein das ist ein Missverständniss. Jede GUI-Operation (abgesehen von InvokeRequired, Invoke, BeginInvoke, EndInvoke und CreateGraphics) MUSS im Gui-Thread erfolgen. Du kannst in einem anderen Thread einer GUI kein Control hinzufügen. Sobald Du das versuchst ist ein Invoke notwendig, und damit ein Wechsel in den GUI-Thread. Das heißt, dass der Thread NICHT "ständig" darauf zugreifen kann.

TobiBob schrieb:

Es werden einige Daten während einer Simulation in ziemlich schneller Abfolge in den Labels angezeigt

Bei hoher Updatefrequenz von berechneten Daten sollte man sich nach dem menschlichen Benutzer richten. Ein Mensch kann wenige Dutzend Änderungen pro Sekunde visuell wahrnehmen. Kognitiv erfassen kann er eine handvoll Änderungen. Wenn überhaupt. Das heißt: Tausend GUI-Updates pro Sekunde sind niemals sinnvoll. Und im umgekehrten Fall: Das, was ein Mensch wahrnehmen kann, belastet eine GUI i.d.R. nicht derart, wie von Dir angesprochen.
Sammele Deine Daten und zeige sie zu geeigneten Zeitpunkten an.

hustbaer

Noch was wichtiges vergessen:

Wenn du Invoke oder BeginInvoke auf eine geschlossene Form (bzw. "geschlossenes" Control) machst, dann fliegt dir eine Exception um die Ohren.

Wenn du den Thread joinst bevor du die Form schliesst, darf das nie vorkommen, d.h. du musst den Fall nicht berücksichtigen.

Wenn du aber die "cancel = true und vergessen" Variante machen willst, dann hast du den Fall sofort. Nämlich dann, wenn du das "cancel = true und vergessen" im "FormClosing" Event machst.

Die einfachste (wenn auch wenig elegante) Möglichkeit ist dann, die aus Invoke fliegende Exception zu fangen, und den Thread zu beenden wenn man was gefangen hat.

hustbaer

TobiBob schrieb:

Es ist ja nur Text-in-drei-labels-schreiben, also ziemlich primitiv einfach - zu einfach, als dass ich Deine Hinweise wirklich voll darauf abtesten könnte, fürchte ich. Aber sie geben mir einen guten Eindruck von "good practice".

Also ich meine sowas:

// Form1.cs
using System;
using System.Threading;
using System.Windows.Forms;

namespace cs10test
{
	public partial class Form1 : Form
	{
		public Form1()
		{
			InitializeComponent();
		}

		private void Form1_Load(object sender, EventArgs e)
		{
			m_label = new Label();
			m_label.Parent = this;
			m_label.SetBounds(10, 10, 200, 30);

			m_cloneButton = new Button();
			m_cloneButton.Parent = this;
			m_cloneButton.SetBounds(10, 60, 200, 30);
			m_cloneButton.Text = "Dolly!";
			m_cloneButton.Click += delegate(object s, EventArgs a)
			{
				// Baaaaaaaaah!
				new Form1().Show();
			};

			m_stopButton = new Button();
			m_stopButton.Parent = this;
			m_stopButton.SetBounds(10, 110, 200, 30);
			m_stopButton.Text = "StopThread()";
			m_stopButton.Click += delegate(object s, EventArgs a)
			{
				StopThread();
			};

			// Wichtig: bevor wir sterben gehen töten wir auch den Thread!
			FormClosing += delegate(object s, FormClosingEventArgs a)
			{
				StopThread();
			};

			// Run, Forest, run!
			m_thread = new Thread(delegate() { ThreadFn(); });
			m_thread.Start();
		}

		private void StopThread()
		{
			// Cancel-Flag setzen
			lock (m_mutex)
				m_cancelFlag = true;

			// Warten bis der Thread weg ist
			if (m_thread != null)
			{
				m_thread.Join();
				m_thread = null;
			}

			if (m_label != null)
			{
				// Frag nicht warum wir das Label löschen wollen würden,
				// ist bloss ein Beispiel dafür was schief gehen könnte wenn man nen Callback bekommt
				// nachdem der Thread schon gar nimmer läuft

				m_label.Parent = null;
				m_label.Dispose();
				m_label = null;
			}
		}

		private void NotificationFunction(int status)
		{
			if (m_thread == null)
				MessageBox.Show("BAM, oida!");

			m_label.Text = status.ToString(); // BAM, oida!
		}

		private void ThreadFn()
		{
			int status = 0;

			for (; ; )
			{
				// Wir laufen bis wir angehalten werden
				lock (m_mutex)
					if (m_cancelFlag)
						return;

				// Wir machen was trallala
				Thread.Sleep(500);
				status++;

				// Neuen Status anzeigen
				BeginInvoke(new Action(delegate() { NotificationFunction(status); }));
			}
		}

		private Button m_cloneButton;
		private Button m_stopButton;
		private Label m_label;

		private Thread m_thread;

		private object m_mutex = new object();
		private bool m_cancelFlag = false;
	}
}

TobiBob

hustbaer schrieb:

MessageBox.Show("BAM, oida!");

Bist Du Österreicher?

Also @ Euch beide, vielen Dank für die ausführlichen Antworten! Ich werd mir das alles morgen in Ruhe durchlesen, wenn ich wacher bin.
mfg, tobi

hustbaer

Ja, bin Österreicher.

TobiBob

µ schrieb:

TobiBob schrieb:

Ich habe jetzt alle Invoke()-Aufrufe durch BeginInvoke() ersetzt.

Das ist eine Lösung. Mit den Konsequenzen die hustbaer angesprochen hat.
Nämlich dass nach der Beendigung des Abbruchcodes und nach Threadende doch noch Aufträge in der GUI ausgeführt werden können.

Und noch eine andere eher ungünstige Konsequenz, wie ich gerade festgestellt habe: (eine eher seehr ungünstige)
Wenn die Übergaben an die GUI zu viele werden, kommt sie nicht mehr hinterher und man bringt den GUI-Thread auf herrliche Weise zum Abstürzen. Das Programm läuft zwar weiter, lässt sich aber dann nur noch über den Tastmanager beenden...!

Ich muss also ne andere Lösung finden. (... s.u.)

µ schrieb:

(...) Nur falls Du im aufrufenden Thread die Ergebnisse synchron erhalten willst, ist EndInvoke gefragt. Kleine Anmerkung: Mit den Methoden BeginInvoke und EndInvoke von Delegaten, kann jede Methode asynchron ausgeführt werden.
(...)
Du kannst in einem anderen Thread einer GUI kein Control hinzufügen.

Ok. Gut zu wissen.

µ schrieb:

Bei hoher Updatefrequenz von berechneten Daten sollte man sich nach dem menschlichen Benutzer richten. (...)

Dann währe es vielleicht eine bessere Lösung, die Daten gar nicht aus dem Workthread heraus anzuzeigen, sondern über einen Timer, der alle 1 oder 1/2 Sekunde die Daten aus dem Objekt ließt und anzeigt? Ich kann bei meiner kleinen Simulation die Geschwindigkeit anpassen. Wenn man also die Daten mitverfolgen will, kann man es langsam laufen lassen. Aber wenn es zu schnell läuft, bringt eine Datenanzeige im 1ms-Takt natürlich auch nix.

@hustbaer: ja, ich weiß, was Du meinst. Könnte auch so aussehen, und das tut den Augen weh:

public partial class Form1 : Form
	{
		private Thread t;
		public Form1()
		{
			InitializeComponent();
		}

		private void Form1_Load(object sender, EventArgs e)
		{
			t = new Thread(new ThreadStart(Start));
			t.Start();
			this.Close();
		}
		private void Start()
		{
			Thread.Sleep(100);
			try
			{
				this.button1.BeginInvoke((MethodInvoker)delegate
				{
					this.button1.Text = "Bam!";
				});
			}
			catch { } // Uuaaahh!
		}
	}