Stoppen mehrer asynchroner Methodenaufrufe

Hallo zusammen,

ich wollte meiner Anwendung eine Suchfunktionalität hinzufügen, die eine Liste definierter Domains nach bestimmten Computern
durchsucht. Aus diesem Grund habe ich zunächst eine einfache Klasse erstellt, über die eine solche Suche asynchron ausgeführt
werden kann.

Die Klassenimplementierung sieht folgendermaßen aus:

class AsyncSearch
    {
        private delegate string[] SearchHandler(string Filter, string Domain);
        private SearchHandler _search;

        private ManualResetEvent _shutdownEvent = new ManualResetEvent(false);

        private bool _stopped = false;
        public bool Stopped
        {
            get { return _stopped; }
        }

        public string[] Search(string Filter, string Domain)
        {
            Random rnd = new Random();
            int val = rnd.Next(1000, 10000);

            // simulate a search period between 1 and 10 seconds
            Thread.Sleep(val);

            // simulate the search results
            val = rnd.Next(50);
            string[] ResultList = new string[val];

            for (int i = 0; i < val; i++)
            {
                if (_shutdownEvent.WaitOne(0)) // stop the thread
                    break;

                ResultList[i] = "_Computer_" + i.ToString();
            }

            return ResultList;
        }

        public IAsyncResult BeginSearch(string Filter, string Domain, AsyncCallback asyncCallbak, object state)
        {
            _search = new SearchHandler(Search);
            return _search.BeginInvoke(Filter, Domain, asyncCallbak, state);
        }

        public string[] EndSearch(IAsyncResult asyncResult)
        {
            return _search.EndInvoke(asyncResult);
        }

        public void Stop()
        {
            _shutdownEvent.Set();
            _stopped = true;
            return;
        }
    }

Zum Starten der Suche und Anzeige der Ergebinsse habe ich ein Form mit einem Button und einer ListView erstellt. Die Suche selbst wird in
einem Hintergrundthread gestartet, der für jede Domain in der Liste die Methode Search() (siehe oben) asynchron aufruft.

Der dazugehörige code sieht so aus:

public partial class Form1 : Form
    {

        bool _searchRunning = false;
        List<AsyncSearch> _listSearches;
        List<string> _listDomains;

        object _sync = new object();

        public Form1()
        {
            InitializeComponent();

            _listDomains = new List<string>();
            _listDomains.Add("domain1.com");
            _listDomains.Add("domain2.com");
            _listDomains.Add("domain3.com");
            _listDomains.Add("domain4.com");

            _listSearches = new List<AsyncSearch>();

        }

        private void button1_Click(object sender, EventArgs e)
        {

            if (!_searchRunning) // if no search is running, start a new one
            {
                _listSearches.Clear();
                listView1.Items.Clear();

                _searchRunning = true;
                button1.Text = "Abort";

                WaitCallback wcRunAsyncADSearch = new WaitCallback(DoSearch);
                wcRunAsyncADSearch.BeginInvoke(textBox1.Text, delegate(IAsyncResult asyncResult)
                {
                    wcRunAsyncADSearch.EndInvoke(asyncResult);

                    MethodInvoker miChangeText = delegate
                    {
                        _searchRunning = false;
                        button1.Text = "Search";
                    };
                    Invoke(miChangeText);

                }, null);

            }
            else // if search is running and Button is pressed again, stop the search by calling AsyncSearch.Stop()
            {
                lock (_sync)
                {
                    foreach (AsyncSearch searchItem in _listSearches)
                        searchItem.Stop();

                    _searchRunning = false;
                    button1.Text = "Search";
                }
            }
        }

        private void DoSearch(object state)
        {
            string searchTerm = (string)state;

            List<WaitHandle> listReset = new List<WaitHandle>();

            lock (_sync)
            {
                foreach (string domain in _listDomains)
                {
                    AsyncSearch asyncSearch = new AsyncSearch();
                    _listSearches.Add(asyncSearch);

                    IAsyncResult resetResult = asyncSearch.BeginSearch(searchTerm, domain, delegate(IAsyncResult asyncResult)
                    {
                        string[] entries = asyncSearch.EndSearch(asyncResult);

                        if (entries.Length > 0)
                        {

                            MethodInvoker miAsyncListAdd = delegate
                            {
                                foreach (string entry in entries)
                                {
                                    if (asyncSearch.Stopped)// true after AsyncSearch.Stop();
                                        break;

                                    ListViewItem listItem = listView1.Items.Add(entry);
                                    listItem.ImageIndex = 0;

                                }
                            }; 
                            Invoke(miAsyncListAdd);

                        }

                    }, null);

                    listReset.Add(resetResult.AsyncWaitHandle);
                }
            }

            WaitHandle.WaitAll(listReset.ToArray()); // wait till all async search calls are finished.
        }
}

Zusätzlich zum Ausführen einer Suche wollte eine Möglichkeit integrieren, um eine laufende Suche abzubrechen und direkt danach
wieder eine neue Suche starten zu können. Die dafür zuständige code zum einleiten des Abruchs einer Suche ist im else Teil des
button1_click events implementiert:

else // if search is running and Button is pressed again, stop the search by calling AsyncSearch.Stop()
            {
                lock (_sync)
                {
                    foreach (AsyncSearch searchItem in _listSearches)
                        searchItem.Stop();

                    _searchRunning = false;
                    button1.Text = "Search";
                }
            }

Es wird also für jedes AsyncSearch object die Methode Stop() aufgerufen, was indirekt die jeweilige async Mehtode beendet.

Der code funktioniert, wenn der Search/Abrot Button mit normaler Geschwindikeit betätigt wird. Drückt man den Button aber mehrmals schnell
hintereinander kann es sein das die Liste mit den AsyncSearch objecten noch leer ist während bereits der Abbruchteil des button_click() events
läuft.

Es ist somit eine Frage des Timings, welcher Thread eben schneller ist bzw. an der Reihe ist damit diese "Abbruchfunktionalität" funktionieren kann.

Die eigentliche Frage ist nun, hat jemand eine Idee, wie diese Timing Problem gelöst werden kann bzw. einen alternativen Vorschlag um hier einen Abbruch
mehrer async. laufender Methoden zu implementieren?

Gruß
cap_Chap

hajb

Du könntest jeweils im Abbruch- und Startteil einen lock auf _listSearches setzten. Vielleicht wäre es aber sogar besser Dein lock (_sync) um den ganzen Rumpf von button1_Click zu legen.

Übrigens würde ich die button1_Click etwas vereinfachen

private void button1_Click(object sender, EventArgs e)
      {
         lock (_sync)
         {
           if (!_searchRunning) // if no search is running, start a new one
              this.startSearch();
           else
              this.stopSearch();
         }
      }

Pass aber auf, dass Dein lock nicht die Performance drückt oder so.

Durch das neue Konstrukt ändert sich im Grunde am Timing Problem der Abbruchfunktion nichts außer dass nun button1_click, zugegeben übersichtlicher ist.

Hab das mal nachgestellt mit folgendem Ergebnis:

Ausgabe Debugger, wenn button 3x schnell hintereinander betätigt wird:

StartSearch
DoSearch
StopSearch
StartSearch
DoSearch
StopSearch
StartSearch
StopSearch // button_click bekommt zweimal direkt hintereinander das lock und erst danach DoSearch !
DoSearch

Programmcode wie folgt angepasst:

private void startSearch() 
{
...
Debug.WriteLine("StartSearch")
...
}

private void stopSearch() 
{
...
Debug.WriteLine("StartSearch")
...
}

private void DoSearch() // siehe oben
{

...
lock(sync) 
{
	Debug.WriteLine("DoSearch");
	foreach(string domain in _listDomains) ...

}
...
}

hajb

cap_Chap schrieb:

Durch das neue Konstrukt ändert sich im Grunde am Timing Problem der Abbruchfunktion nichts außer dass nun button1_click, zugegeben übersichtlicher ist.

Was nicht ganz unwesentlich ist. Ich hatte Dich falsch verstanden bzw. Deinen Code nicht wirklich gelesen.

Du startest die Suche asynchron? Das heißt, ein click-button-event ist schon vorbei und ein neues ist angekommen, während die Suche noch startet. Richtig?

Du musst eben zusehen, dass der Start vollzogen ist bevor Du die Kontrolle abgibst. Monitore? Nee, böse. Aber Du könntest in deiner DoSearch nochmal fragen, ob nicht schon abgebrochen wurde (_searchRunning == false?) und dann garnicht erst anfangen.

Aber irgendwie scheint das unsauber zu sein. Besser wäre sowas wie ein BackgrundWorker, dem man sagen kann: Start, stop. Und dann immer genau das passiert. Zumindest würde ich die Logik aus der GUI raus ziehen.

Viel Erfolg

Danke zunächst mal!

hajb schrieb:

Du startest die Suche asynchron? Das heißt, ein click-button-event ist schon vorbei und ein neues ist angekommen, während die Suche noch startet. Richtig?

Genau das ist das Problem.

hajb schrieb:

Monitore? Nee, böse.

Das Monitoring war in diesem Fall nur dazu gedacht den Abbruchteil und den Aufruf von DoSearch()
zu synchronisieren. Soll heißen dass erst abgebrochen werden kann, wenn die Suche vollständig läuft, was ja auch Sinn macht.
Ohne die lock() Anweisung könnte es theoretisch passieren, dass aus einer Liste mit mehreren Domains nur für einen
Teil die AsyncSearch Klasse instanziert wird und somit auch nur dieser Teil gestoppt werden kann.

Beispiel (ohne lock()):

1. GUI-THREAD startet DoSearch
2. Scheduler führt DoSearch-THREAD aus und instanziert AsyncSearch in foreach() für einen Teil der Domains.
3. Scheduler führt nach Ablauf des Zeitfensters wieder den GUI-THREAD aus
4. Abbrechen wird betätigt.
5. Alle async. gestarteten Suchläufe, die jetzt in _listSearches sind, werden gestoppt.
6. DoSearch-THREAD arbeitet die foreach() mit den restlichen Domains ab.
7. Suche läuft für die restlichen Domains obwohl abbrechen betätigt wurde.

hajb schrieb:

Aber Du könntest in deiner DoSearch nochmal fragen, ob nicht schon abgebrochen wurde (_searchRunning == false?) und dann garnicht erst anfangen.

Ja, so könnte man das Problem umgehen.

Ich muss dir allerdings Recht geben, wenn du sagst dass das unsauber ist, mir gefällt das auch noch nicht so ganz.

hajb schrieb:

Besser wäre sowas wie ein BackgrundWorker, dem man sagen kann: Start, stop. Und dann immer genau das passiert.

Selbst wenn ich hier mit der Klasse BackgroundWorker arbeiten würde, löst das immer nocht nicht das am Anfang beschriebene Problem.

Oder wie sollte BackgroundWorker hier deiner Meinung nach zum Einsatz kommen?

DoSearch mit Hilfe von BackgroundWorker starten und stoppen oder nur die async. Suchläufe?

In jedem Fall muss eine Liste mit BackgroundWorker-Objecten verwaltet und für jeden
BackgroundWorker CancelAsync() aufgerufen werden, was aber im Vergleich zum aktuellen Code
keine Änderung in Bezug auf die button_click Events herbeiführt.

hajb

cap_Chap schrieb:

hajb schrieb:

Monitore? Nee, böse.

Das Monitoring war in diesem Fall nur dazu gedacht den Abbruchteil und den Aufruf von DoSearch()
zu synchronisieren. Soll heißen dass erst abgebrochen werden kann, wenn die Suche vollständig läuft, was ja auch Sinn macht.
Ohne die lock() Anweisung könnte es theoretisch passieren, dass aus einer Liste mit mehreren Domains nur für einen
Teil die AsyncSearch Klasse instanziert wird und somit auch nur dieser Teil gestoppt werden kann.

Ich meinte nicht, dass locks böse sind. Ich meinte diese hier: http://msdn.microsoft.com/en-us/library/system.threading.monitor(VS.71).aspx. Man muss sehr aufpassen, dass man sich keinen Deadlock einfängt. Deswegen pauschal erstmal böse, aber es gibt Fälle wo man's braucht.

cap_Chap schrieb:

Ich muss dir allerdings Recht geben, wenn du sagst dass das unsauber ist, mir gefällt das auch noch nicht so ganz.

hajb schrieb:

Besser wäre sowas wie ein BackgrundWorker, dem man sagen kann: Start, stop. Und dann immer genau das passiert.

Selbst wenn ich hier mit der Klasse BackgroundWorker arbeiten würde, löst das immer nocht nicht das am Anfang beschriebene Problem.

Oder wie sollte BackgroundWorker hier deiner Meinung nach zum Einsatz kommen?

DoSearch mit Hilfe von BackgroundWorker starten und stoppen oder nur die async. Suchläufe?

In jedem Fall muss eine Liste mit BackgroundWorker-Objecten verwaltet und für jeden
BackgroundWorker CancelAsync() aufgerufen werden, was aber im Vergleich zum aktuellen Code
keine Änderung in Bezug auf die button_click Events herbeiführt.

Sorry, ich meinte nicht _den_ BackgroundWorker aus System.ComponentModel, sondern eher als Konzept. Er soll die Arbeit machen. Die GUI braucht sich nicht zu kümmern wie. Wenn sie "Start" sagt, dann startet er, bei "Stop" hört er auf.