C# thread safety

[opicron]

Hiya,


Zou iemand mij met het volgende probleem kunnen helpen?

MainForm:

Creeërd eigen instantie van een class. Aan deze class word een functie via een delegate meegegeven. Deze functie zal een progress bar value aanpassen aan de waarde die deze terug krijgt. (code below)

code:

        public void SetProgress(int value)
        {
            progressBar1.Value = value;
        }

        private void button1_Click(object sender, EventArgs e)
        {
            dsProgress.ds = new TestDataSet();
           
            dsProgress.ProgressEvent += new DataSetProgressTracker.ProgressEventHandler(SetProgress);

            dsProgress.StartProgressTracker();
        }

Klasse:

Deze classe start een thread, en roept de in de MainForm meegegeven functie (van de delegate) aan. De functie van de main form word aangeroepen, echter krijg ik een exception bij het toewijzen van de value aan de progress bar. (code below)

code:

    class DataSetProgressTracker
    {
        public TestDataSet ds = null;

        private Thread dsFillThread = null;
        private Thread dsProgressThread = null;
        
        public delegate void ProgressEventHandler();
        public event ProgressEventHandler ProgressEvent;

        int intTotalRows = 0;
        int intProgress = 0;

        bool IsLoaded = false;

        private int GetRecordCount()
        {
            return 500;
        }

        private void SetProgress(int value)
        {
            intProgress = value;
            ProgressEvent(value);
        }

        private void ProgressTrackCallBackFunction()
        {
            while (this.IsLoaded == false)
            {
                int value = (ds.TestData.Count * 100) / this.intTotalRows;


                SetProgress(value);
                
                
                Thread.Sleep(50);
            }
        }

        private void DataSetFillCallBackFunction()
        {
            IsLoaded = false;
            ds.Fill();
            IsLoaded = true;
        }

        public void StartProgressTracker()
        {
            intTotalRows = GetRecordCount();

            Thread DataSetProgressThread = new Thread(new ThreadStart(ProgressTrackCallBackFunction));
            Thread DataSetFillThread = new Thread(new ThreadStart(DataSetFillCallBackFunction));
            
            DataSetProgressThread.Start();
            DataSetFillThread.Start();
        }
    }

Deze uitzondering word getriggerd:

Cross-thread operation not valid: Control 'progressBar1' accessed from a thread other than the thread it was created on.

Nu kan ik deze value wijzigen om de applicatie te laten werken, maar ik vind dat geen mooie oplossing:

code:

CheckForIllegalCrossThreadCalls = false;

Thx!

[ Voor 6% gewijzigd door opicron op 15-08-2006 14:25 ]

[PhysicsRules]

In principe mag een thread niet aan objecten komen van een andere thread. Je zult dus een methode moeten vinden waarop de Progress thread aan je hoofdthread doorgeeft dat de progressbar geüpdate moet worden. Hoe, dat weet ik niet, dus ik ben niet echt een hulp...

[Flard]

Het probleem isdat je niet in een andere thread iets mag doen met de GUI dan in de thread waarin de GUI gemaakt is.
Dat is de reden dat iedere control ook een InvokeRequired parameter heeft om te kijken of je moet invoken, en heeft dan ook de Invoke functie.

In principe kun je dus het volgende aan de GUI-kant doen.

C#:

        delegate void SetProgressDelegate(int value);
        public void SetProgress(int value)
        {
            if (this.InvokeRequired)
            {
                this.Invoke(new SetProgressDelegate(SetProgress), value);
                return;
            }
            progressBar1.Value = value;
        }

(Voor bepaalde code, die je veel gaat hergebruiken, kun je ook aan de kant die de event opgooit kijken of het target sychronized is. Dan heb je dus alleen een bepaald mechanisme nodig aan de event-kant. Hoe dat precies werkt moet je even op internet opzoeken, ik heb op het moment geen code bij de hand)

Ook denk ik dat je dit best wel had kunnen vinden op GoT of op google...

[opicron]

@PhysicsRules:

De thread komt niet aan objecten van de MainForm.

Er word alleen een event getriggered, met daarin de nieuwe waarde die de control op de mainform moet krijgen.

[ Voor 5% gewijzigd door opicron op 15-08-2006 14:45 ]

[opicron]

Thx Flard,

Jouw oplossing werkt prima.

Ik had echter gehoopt dat ik geen delegate en/of invoke checking in het MainForm had hoeven doen. En dat ik dit puur in de class kon plaatsen.

Op google had ik idd veel van deze oplossingen gevonden, ik was alleen van mening dat ik dit in de class kon plaatsen. Wat mij dus ook niet lukte.

Nu kan ik iig verder, maar ik wil zeker nog deze code aan de event trigger kant (in class) hebben. Ik ga flink verder googlen.

[ Voor 46% gewijzigd door opicron op 15-08-2006 14:53 ]

[Flard]

Ahhh... ik heb nog een handige thread gevonden, waarin de oplossing staat die je denk ik nodig hebt, en met wat ik normaal gebruik:

whoami schreef op zaterdag 13 mei 2006 @ 10:53:
Optie 3 is de beste oplossing.

als je je applicatie een interface laat implementeren, dan bouw je al een afhankelijkheid in. (Een applicatie kan trouwens geen interface implementeren; een class wel. Je zou dan dus de class die je connection-class gebruikt een interface moeten laten implementeren, maar ik zou het niet doen).
Optie 2 is gewoon niet goed vind ik; het is beter dat de connectie-class laat weten als er iets aankomt, of als er iets gebeurt, en dan kom je bij optie 3 uit: gebruik maken van events.

Bij optie 3 kan je dus een eigen delegate maken, die je gebruikt als event.
Maak bv een dergelijke delegate:

code:

public delegate MessageEventHandler( string message );

In je class kan je dan een event maken voor dit type:

code:

class TheClass
{
    public event MessageEventHandler MessageReceived;

    protected void OnMessageReceived( string message )
    {
         if( MessageReceived != null )
         {
             // invoke the event
             Delegate[] delegates = MessageReceived.GetInvocationList();
             foreach( Delegate del in delegates )
             {
                  ISynchronizeInvoke bliep = del.Target as ISynchronizeInvoke;

                  if( bliep != null && bliep.InvokeRequired )
                  {
                         // eventhandler moet op een andere thread uitgevoerd worden
                         bliep.Invoke (del, new object[]{message});
                  }
                  else
                  {
                         del.DynamicInvoke (new object[] {message});
                  }
             }
         }
    }
}

In je class kan je dan aangeven dat die event moet geraised worden.

code:

public class TheClass
{
    ....
    public void AcceptPackage( .... )
    {
        ..
        OnMessageReceived ("packet received.");
    }
}

En de class die je connection class gebruikt kan zich 'abonneren' op dat event:

code:

TheClass c = new TheClass();
c.MessageReceived += new MessageEventHandler (TheClass_MessageReceived);
....
public void TheClass_MessageReceived( string message )
{
   MessageBox.Show (message);
}

Zo ongeveer; let wel, alles uit de losse pols, dus er kan wel hier en daar een foutje inzitten, maar dit zou toch een idee moeten geven.

<sub>Voor de puristen: de MessageEventHandler zou eigenlijk 2 parameters moeten hebben ipv 1: nl. een object sender, en een class die afgeleid is van de EventArgs class. Die kan je dan bv MessageEventArgs noemen, en die bevat dan een member 'Message' van het type string.

Note: als je geen .NET 2.0 gebruikt, dan zal die ISynchronizeInvoke interface (zit i/d System.ComponentModel dacht ik), niet herkend worden, omdat die niet bestaat in .NET 1.x.
In dat geval zal je ipv die ISynchronizeInvoke System.Windows.Forms.Control moeten gebruiken. (Dan moet je jammergenoeg wel een reference naar de System.Windows.dll oid leggen).</sub>

[opicron]

Ja, dit probeer ik voor elkaar te krijgen.

Probeer nu de protected void OnMessageReceived( string message ) om te bouwen zodat deze gaat werken. Is niet ff 123 te doen merk ik.

Die benaming van de OnMessageRecieved vat ik ook niet helemaal. Ik wil in de class juist een event triggeren, oftewel een message sturen, en ik geloof dat dat in de voorbeeld class ook zo is.

[ Voor 34% gewijzigd door opicron op 15-08-2006 15:08 ]

[Flard]

opicron schreef op dinsdag 15 augustus 2006 @ 15:04:
Ja, dit probeer ik voor elkaar te krijgen.

Probeer nu de protected void OnMessageReceived( string message ) om te bouwen zodat deze gaat werken. Is niet ff 123 te doen merk ik.

Volgens mij staat er een fout in je aanvankelijke code

C#:

        public delegate void ProgressEventHandler();
        public event ProgressEventHandler ProgressEvent;

        private void SetProgress(int value)
        {
            intProgress = value;
            ProgressEvent(value);
        }

Dit kan niet, aangezien ProgressEventHandler parameterloos is...

Als ProgressEventHandler wel een parameter 'int value' heeft zou het als volgt worden:

C#:

private void SetProgress(int value)
{
    intProgress = value;
    if (ProgressEvent != null)
    {
        // invoke the event
        Delegate[] delegates = ProgressEvent.GetInvocationList();
        foreach( Delegate del in delegates )
        {
            ISynchronizeInvoke bliep = del.Target as ISynchronizeInvoke;

            if( bliep != null && bliep.InvokeRequired )
            {
                // eventhandler moet op een andere thread uitgevoerd worden
                bliep.Invoke (del, new object[]{value});
            }
            else
            {
                del.DynamicInvoke (new object[] {value});
            }
        }
    }
 }

[opicron]

Flard


Bedankt voor de zeer snelle en accurate hulp!

Een probleem waar ik absoluut geen kennis van had heb ik nog nooit zo snel kunnen oplossen.

Maw, het werkt nu prima!

Moest alleen ff zoeken naar de namespace van ISynchronizeInvoke, System.ComponentModel.

*edit* Ik ben echt niet wakker ofzo, stond in kleine letters onder vb. code welke namespace het was.

[ Voor 39% gewijzigd door opicron op 15-08-2006 15:30 ]

[PhysicsRules]

opicron schreef op dinsdag 15 augustus 2006 @ 14:38:
@PhysicsRules:

De thread komt niet aan objecten van de MainForm.

Er word alleen een event getriggered, met daarin de nieuwe waarde die de control op de mainform moet krijgen.

Ik heb er ook weer wat bijgeleerd. Leuk probleem

[whoami]

Ik zie dat je gebruik maakt van .NET 2.0, waarom maak je dan gewoon geen gebruik van de BackGroundWorker class die al deze shizzle voor jou oplost ?

[opicron]

Top - ga ik ook direct naar kijken. Hoe meer ik hiervan oppik, des te beter.

*edit*

Ik heb nu de background worker geimplementeerd, en als ik niet de invoke code zoals in vorig bericht implementeer krijg ik dezelfde uitzondering als bij de normale thread class.

*edit*

Dit kwam dus omdat ik niet de ProgressReport event en functie gebruikte.

[ Voor 137% gewijzigd door opicron op 16-08-2006 16:59 ]

[Flard]

BackgroundWorker heeft ook een ReportProgress functie volgens mij...


(Wat dom trouwens dat ik de BackgroundWorker ben vergeten )

[whoami]

opicron schreef op woensdag 16 augustus 2006 @ 08:29:
Top - ga ik ook direct naar kijken. Hoe meer ik hiervan oppik, des te beter.

*edit*

Ik heb nu de background worker geimplementeerd, en als ik niet de invoke code zoals in vorig bericht implementeer krijg ik dezelfde uitzondering als bij de normale thread class.

Het maakt op zich dus niet veel uit welke van de twee je gebruikt, althans in mijn project.

BackgroundWorker heeft wel voordelen ten opzichte van Thread:

- Een thread is na completion opnieuw te starten (dus één keer new() aanroepen)
- Er word een complete event getriggerd als de thread klaar is

Echter ook nadelen:

- Geen params mee te geven aan de event triggers, zover ik in een paar uur heb ontdekt.

De background worker class is net gemaakt om ervoor te zorgen dat je zelf niet meer hoeft te bepalen of het rapporteren van de progress bv geinvoked moet worden of niet.
Als je de BackGroundWorker dus goed gebruikt, dan zou het wel moeten werken. Je moet natuurlijk wel goed gebruik maken van de events die de BackGroundWorker heeft. (ReportProgress bv, en WorkCompleted, oid)

[opicron]

Ahh, die property heb ik gemist.

Het werkt idd nu wel zonder zelf de invoke thread te bepalen.