Best way to stop a Thread, when activity gets destroyed?

srensren DKMember

Hey

I got this thread i use to update a textView every 5 second:

t1 = new System.Timers.Timer ();
t1.Elapsed += new ElapsedEventHandler(OnTimeEvent);
t1.Interval = 5000;
t1.Start();

i got an OnDestroy(), where i currently do t1.Stop().

This can create an error were the thread haven't finished its work, and therefor the app crashes. Do i need to wait for the thread to finish? and if i do that wont the app, have to wait for the thread to finish before it changes activity?

What i would love to get is a way to make my app change activity within 0.5sec.

Posts

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭
    edited May 2013

    By thread I assume you mean the Timer callback.

    It depends on the code you are executing within the callback. If it is critical that it runs to completion before leaving the activity, use .Dispose(WaitHandle).

    If the callback can take a while and it's more important that you can destroy the activity immediately, add checks in the callback code surrounding anything that accesses the activity, to ensure it's actually still around while executing. You will also want to put lock statements in, in that case. Careful, there be dragons ;)

  • srensren DKMember

    Yes its a Timer callback, and i don't want to wait for it to finish. the important thing is that ill be able to change activity without w8ing for the timer to finish. This i my timer event

        private void OnTimeEvent(object source, ElapsedEventArgs e)
    
        {
    
            GetProductionDays ();
    
            ThreadPool.QueueUserWorkItem (o =>this.Activity.RunOnUiThread (() => textSite.Text = productionHolder.log.entries[0].values[0].value.ToString()));
    
    
    
        }
    

    what checks is it you want me to create?

  • srensren DKMember
    edited May 2013

    I've done this in my OnDestroy

    public override void OnDestroy(){

            t1.Dispose ();
    
            base.OnDestroy();
    
        }
    

    The thread stops, but i get the nullreferance on my RunOnUIThread since it dosent exsists anymore. What checks can i create to resolve this?

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭

    Basically check if the Activity is non-null and in a valid state.

    But why are you creating a new thread inside the callback?

    Also, it looks like productionValues is an out-of-scope variable - this can lead to race conditions in theory.

    I think there is a much simpler way for what you are doing - but I would need to know what it is that you are doing inside GetProductionDays().

  • srensren DKMember

    ThreadPool.QueueUserWorkItem aint creating new threads as far as i can see in the app output i get 3 threads cause i got 3 objects calling the itmer.

    GetProductionDays() is used to call a webrequest(that takes 30-> secs to get a response) looks like this:

        void GetProductionDays(){
    
            //Sends the webRequest
    
            HttpWebRequest httpWebRequest = (HttpWebRequest)WebRequest.Create("http://webservice.mita-teknik.com/webservice/api/datalog.json/production/day?AssocID="+login.assocID+"&Targets="+site.sites[count].id+"&Days=5");
    
            httpWebRequest.Method = WebRequestMethods.Http.Get;
    
            httpWebRequest.Accept = "application/json";
    
            //Gets the webresponse
    
            HttpWebResponse webR = (HttpWebResponse)httpWebRequest.GetResponse ();
    
            //Converts the response to string
    
            using (StreamReader sr = new StreamReader(webR.GetResponseStream())) 
    
            {
    
                //deserializes a JSON object to .NET object
    
                productionHolder = JsonConvert.DeserializeObject<FiveDaysProductionHolder>(sr.ReadToEnd ());
    
            }
    
        }   
    
  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭

    ThreadPool.QueueUserWorkItem() uses the ThreadPool to allocate a background thread - this may create a new thread or reuse an existing one. But why use this at all here, why not just call RunOnUiThread() directly?

    Also, are you saying that you are calling a method every 5 seconds that can take more than 30 seconds to complete?

    Correct me if I'm wrong, but from the sound of GetProductionDays it seems like this returns a result that does not change frequently. Are you using the timer in fact to just poll for when the result becomes available, and then dynamically update once it does?

    If so (again, correct me if I'm wrong), you should just remove all the timer stuff - instead, use the asynchronous WebRequest.BeginGetResponse(), which you can pass a callback in which updates your textview once the result is available (you do need RunOnUIThread in there, but that is all).

  • srensren DKMember
    edited May 2013

    I can surely delete ThreadPool.QueueUserWorkItem(), alrdy tryed it and atm it changes nothing(so ill stop using it).

    What i want my app to do, is to update a textView with a value that i get from GetProductionDays(it doesn't have to be every 5 sec it could be ones every minute, the important thing is that it keeps updating itself every X sec/minute), since the data i pull with GetProductionDays, keeps updating, i need to run that method to, so i always show the newest values.

    So whats your advice?

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭

    The bottleneck is in the web service call. First of all you will want to make sure that you do not call the web service while it's still running. You will just end up running the same request multiple times concurrently.

    Lots of ways to go about this. You could use a field to check whether sufficient time has passed since finishing the last webservice request, before actually entering the rest of the callback. Make sure to surround the service call with a try-finally that actually resets the field. I am assuming that the service request generates significant load on the server, if it takes that long to complete. It might also be wise to implement an exponential backoff of some kind, to prevent flooding the server.

    You could also implement a Service module that calls the webservice every so often, and stores the result and signals your activity once updated.

  • srensren DKMember

    Can you give any examples, I'm not used to webrequests, all i know is that the code i got atm works:).

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭
    edited May 2013

    very very simplified pseudo code:

    TIME_SINCE_LAST_RESPONSE = 0
    LAST_RESULT = NULL
    Callback:
        IF TIME_SINCE_LAST_RESPONSE > 60secs THEN BEGIN
            LAST_RESULT = SERVICE_CALL()
            UPDATE_TEXTVIEW_WITH_RESULT
        END
    
  • srensren DKMember

    the callback being a timer?

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭

    yep

  • srensren DKMember

    I've tried this:

    private void OnTimeEvent(object source, ElapsedEventArgs e)
    {
    int timeSinceLastResponse = 0;
    if (timeSinceLastResponse > 60) {
    GetSetProductionDays ();
    }
    }

    But that would reset the = all the time, and if i set int timeSinceLastResponse = 0; outside the timer it would call it after >60 and then it will call it all the time just like my old code.

    I'm sure i missed something, but what^^?

    btw GetSetProductionDays (); is i new method i made it got one method to set the text and one that calls the webrequest

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭

    timeSinceLastResponse should be scoped OUTSIDE the callback. And you need to reset it once you have the result etc. I'm hoping the very very rough pseudo code was enough to give you an idea.

  • srensren DKMember

    Okey i think it works now^^(seems to be^^), but i still got a problem with the null reference where it tries to set the textView AFTER the activity is closed.

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭

    You will need to ensure that you check that the activity is still around, inside the callback, as mentioned earlier. Or wrap it in a try catch.

  • srensren DKMember

    if i ensure that my activity is still around wouldn't take make my app wait for the thread to finish? = i wont change activity before the thread is done working?

    i tried a try catch(maybe i did it wrong) but i get the same error EVEN though I'm trying to catch it, it wont let me catch it.

    try {

                if (DateTime.Now > dateTime.AddSeconds(30)) {
    
                    GetSetProductionDays ();
    
                }
    
            } catch (Exception ex) {
    
    
    
            }
    
  • srensren DKMember

    Forgot to use throw(ex);

    Thanks for all your help, REALLY, appreciate it.

  • ChrisHonselaarChrisHonselaar NLBeta ✭✭✭

    The try-catch should be INSIDE the RunOnUIThread bit. Can't catch an exception in a different thread ;)
    But preferably use a check before resorting to try-catch, even though it should work it's a bit ugly.

Sign In or Register to comment.