3

I have big problem with understanding multi threading in my application and because of that finding a bug. I've checked I think all possibilities and still I am getting various (sometimes unexpected) errors.

Maybe someone here will be able to advice me, what I should do.

In my project I am using two external libraries:

  • GraphView - provides views for graph drawing
  • EventBus - provides interface for easy communication between app components

As for the app it has the structure like this:

           MainActivity
            /        \
           /          \
        Thread        Fragment
   (ProcessThread)   (GraphFragment)

The idea is that ProcessThread computes data and provides constant stream of values to GraphFragment throught EventBus. In GraphFragment I have one Series required by GraphView.

To update graphs in real time according to the example I need to make a new Runnable so I did one:

private class PlotsRun implements Runnable{

        @Override
        public void run() {
            mSeries1.appendData(new DataPoint(counter, getRandom()), true, 100);
            counter++;
            mHandler.post(this);
        }
}

and when I start it from fragment's onResume() method everything is working like a charm.

Unfortunately as I've mentioned I am using external data from another thread. To get it inGraphFragment I am using (according to the documentation) onEventMainThread() method.

And in here no matter what I'll do I can't pass data to update my graph in PlotsRun object. So far I've tried:

  • using Queue - add value in onEventMainThread and get in PlotsRun. It turned out that runnable is reading faster than method is able to update queue.
  • creating various buffers - the result is quite this same as with Queue.
  • calling mSeries1.appendData(new DataPoint(counter, getRandom()), true, 100); directly from onEventMainThread - at some point it gets freez.
  • creating onEvent() method inside my runnable and call from there mHandler.post() - it is blocking UI and updates looks like snapshots.
  • using everything mentioned with or without synchronized() block.

What is quite difficult for me to understand is this runnable which is working correctly (at some point).

As it is said on official Android blog you can't update UI from non UI thread. This is why I can't use another thread inside GraphFragment. But when I've checked my runnable it is running on main thread (UI). This is why I can't create infinite while loop there instead have to call mHandler.post(this).

And still it behaves like another thread because it is faster (called more frequently) then onEventMainThread method.

What can I do to be able to update my graphs (or where I should look) using data from ProcessThread?

EDIT1:

Answering on @Matt Wolfe request I am including what I think is the most important part of a code for this problem with all required variable shown how they are declared. It is very simplified example:

MainActivity:

private ProcessThread testThread = new ProcessThread();

 @Override
    protected void onResume() {
        super.onResume();
        testThread.start();
    }


    private class ProcessThread extends Thread{
        private float value = 0f;
        private ReadingsUpdateData updater = new ReadingsUpdateData(values);
        public void run() {
            while(true) {
                value = getRandom();
                updater.setData(value);
                EventBus.getDefault().post(updater);
            }
        }
    }

GraphFragment:

private LineGraphSeries<DataPoint> mSeries1;
    long counter = 0;
    private Queue<ReadingsUpdateData> queue;

    @Override
    public void onResume() {
        super.onResume();
        mTimer2.run();
    }

    public void onEventMainThread(ReadingsUpdateData data){
        synchronized(queue){
            queue.add(data);
        }
    }

    private class PlotsRun implements Runnable{

        @Override
        public void run() {
            if (queue.size()>0) {
                mSeries1.appendData(new DataPoint(counter, queue.poll()), true, 100);
                counter++;
            }
            mHandler.post(this);
        }
    }

The if in runnable is added for protection because of this to fast reading problem. But it shouldn't be here because there should always be something (at least I expect that).

One more thing to add - when I put simple Log.d and counting variable inside onEventMainThread it was updating and displaying it's value correctly, but unfortunately logcat isn't main UI.

EDIT2:

This is mainly response for @MattWolfe comment

The mHandler is just variable declared and created in GrapgFragment:

private final Handler mHandler = new Handler();
private Runnable mTimer2;

Yes, that is right I am using mHandler.post() without any delay. I'll try using some delay to see if there is any difference.

What I didn't mention earlier is that the ProcessThread is providing also data to other fragments - don't worry they don't interfere with each other or share any resources. This is why I am using EventBus.

EDIT3:

This a code that I've used as my another idea with another thread in GraphFragment and runOnMainThread method:

private MyThread thread = new MyThread();

    private class MyThread extends Thread {
        Queue<ReadingsUpdateData> inputList;
        ReadingsUpdateData msg;

        public MyThread() {
            inputList = new LinkedList<>();
        }

        public void run() {
            while(true) {
                try{
                    msg = inputList.poll();
                } catch(NoSuchElementException nse){
                    continue;
                }
                if (msg == null) {
                    continue;
                }
                getActivity().runOnUiThread(new Runnable() {
                    @Override
                    public void run() {
                        mSeries1.appendData(new DataPoint(counter, getRandom()), true, 100);
                        counter++;
                    }
                });
            }
        }

        public void onEvent(ReadingsUpdateData data){
            inputList.add(data);
        }
    }

Unfortunately, it isn't working neither.

Community
  • 1
  • 1
sebap123
  • 2,541
  • 6
  • 45
  • 81
  • Have you looked into the runOnUIThread method? http://stackoverflow.com/questions/11140285/how-to-use-runonuithread – Proxy32 May 22 '15 at 18:40
  • This is pretty hard to figure out what exactly your code is trying to do. If you want to feed a large data set into your fragment and update the UI based on this data set you need to synchronize access to the data set. This means you need a synchronized block around anywhere you append/remove data from the dataset in both threads. – Matt Wolfe May 22 '15 at 18:41
  • You need to show more of the code for the fragment and the background task so we get a better idea of how/when you're accessing the data. If you can setup a simplified example that would be best. – Matt Wolfe May 22 '15 at 18:45
  • If I understand you create values that you store into mSeries1 from one thread and you need to pass it to another thread (the main one)? Secondly, when you receive your values, do you have a view to inflate,update ? –  May 22 '15 at 18:47
  • @MattWolfe I have updated question with basic example code. – sebap123 May 22 '15 at 19:08
  • Can you show your creation of mHandler? You just need to create that with the main looper. If your queue is empty you just keep re-posting the PlotsRun runnable with no delay? Might peg your cpu. I'd suggest using postDelayed instead with a short delay. Also I would personally change this a bit such that you don't post the ReadingsUpdateData onto the main thread since you aren't doing anything with the UI there anyways. Instead just post it on the thread it was generated in, synchronize access to the queue like you did, and then from there you could use mHandler to post the PlotsRun runnable. – Matt Wolfe May 22 '15 at 19:55
  • @MattWolfe I've added the requested part. As for your suggestion - can you please write it as an answer because I'm not getting everything what you are saying. – sebap123 May 22 '15 at 20:42
  • @Proxy32 Yes I did try this - take a look at edit3 section. – sebap123 May 23 '15 at 10:47

4 Answers4

3

First of All,

The runnable part in your followed example is just for the sake of animating realtime data updating, You can choose to call appendData() without creating a new runnable. You need to call appendData() from main thread though.

Secondly,

You can call the appendData() function directly from your onEventMainThread function, but as you pointed out that this approach sometimes hangs the UI, One possible reason for this behaviour is that you are probably posting events too frequently, Updating UI too frequently would ultimately hang the UI at some point. You can do the following to avoid this:

Updating UI too frequently might also hang the UI, Here's a Solution:

Put some logic in ProcessThread to save last sent event time and compare it before sending a new one and if the difference is less than 1 second than save it for sending later and when the next computation is done, compare the time again, if it is greater than 1 second now, than send the events in array or may be send just the latest event, as the latest computation can represent the latest state of graph right?

Hope that helps!

Edit: (in response to Comment 1 & 2)

I am not sure what you tried may be posting your updated code would give a better idea. but I think you tried to implement time check functionality in onEventMainThread or in PlotsRun runnable, is that correct? If yes than I am afraid that wouldn't be of much of help for you. What you need to do instead is to implement this time checking check inside ProcessThread and only post new event if threshold time is reached. For following reasons:

1- EventBus on the backend automatically creates a new runnable and calls onEventMainThread in it. So, handling time check inside ProcessThread would spawn less unwanted runnables into the memory which would result in less memory consumption.

2- Also no need to maintain queue and spawn new runnables, just update the data in onEventMainThread.

Below is a bare minimum code to provide proof of concept only, You would need to update it according to your needs:

ProcessThread class:

private class ProcessThread extends Thread{
    private static final long TIME_THRESHOLD = 100; //100 MS but can change as desired
    private long lastSentTime = 0;
    private float value = 0f;
    private ReadingsUpdateData updater = new ReadingsUpdateData(values);
    public void run() {
        while(true) {
            if (System.currentTimeMillis() - lastSentTime < TIME_THRESHOLD) {
                try {
                    Thread.sleep(TIME_THRESHOLD - (System.currentTimeMillis() - lastSentTime));
                } catch (InterruptedException e) {}
            }

            value = getRandom();
            updater.setData(value);
            EventBus.getDefault().post(updater);
            lastSentTime = System.currentTimeMillis();
        }
    }
}

onEventMainThread method:

public void onEventMainThread(ReadingsUpdateData data){
    mSeries1.appendData(new DataPoint(counter, data), true, 100);
    counter++;
}
Asif Mujteba
  • 4,596
  • 2
  • 22
  • 38
  • I have to admit that your idea looks like it might work. I've added basic logic checking if difference between received data is more than 50 milliseconds. It worked out great with my runnable. **But** I don't know why the memory allocated is still increasing. With runnable nothing like this happen. With thread it is very slow increase but it is. Have you got any idea what might be causing it? – sebap123 May 25 '15 at 19:19
  • It looks like if I will add this time checking logic to my `ProcessThread` everything works great, while adding it to the `onEventMainThread` end in memory allocation increase. If you are able can you please explain it? – sebap123 May 25 '15 at 19:28
  • @sebap123 I have edited my answer, check my response to your comments there and let me know! – Asif Mujteba May 26 '15 at 11:11
  • Thank you very much for explanation. Your answer fixes my issue, but to be honest it is only a part of bigger problem. I would be very glad if you could take a look at it: http://stackoverflow.com/questions/30458860/unexpected-behavior-of-same-methods-in-different-threads – sebap123 May 26 '15 at 12:42
1

Your PlotsRun is in fact too quick: as soon as it finishs its execution, it requeues itself on the main thread loop execution by calling mHandler.post(processPlots);.

First, you need to make your data buffer independent of the data collector and the data visualizer: make an object that can receive (from collector) and deliver (to visualizer) data. So, each component can works quite independently. And your data object is independent of any thread. Your data collector can push data to your data object when it needs, and your main thread can query your data object based on a regular timer.

Then, put a lock on this buffer so that neither of the 2 other objects that need to access the data buffer can do it simultaneously (that will result in a crash). This lock can be a simple synchronized in the method declaration.

That should insure that your app doesn't crash because of concurrency access (that should be your main problem I think).

Then, you can start to optimize your data object by creating additional buffers to store temporary data if main data collection is already in use when new data arrive, or make a copy of actual data for it to be always available to main thread even when new data is currently being added when the main thread query for values.

Nicolas Buquet
  • 3,880
  • 28
  • 28
0

I would setup something like this:

public class MainActivity extends Activity {

 private class ProcessThread extends Thread{
        private float value = 0f;
        private ReadingsUpdateData updater = new ReadingsUpdateData(values);
        public void run() {
            while(true) {
                value = getRandom();
                updater.setData(value);
                EventBus.getDefault().post(updater);
            }
        }
    }    

    @Override
    protected void onResume() {
        super.onResume();
        testThread.start();
    }

}



public class GraphFragment extends Fragment {

  private Handler mHandler;
  private Queue<ReadingsUpdateData> queue;

  @Override
  public void onActivityCreated(Bundle state) {
    super.onActivityCreated(state);
    mHandler = new Handler(Looper.getMainLooper());
  }

  public void onEvent(ReadingsUpdateData data){
    synchronized(queue){
        queue.add(data);
    }

    if (mHandler != null) {
       mHandler.post(processPlots);
    }
  }

  //implement pause/resume to register/unregister from event bus


 private Runnable processPlots = new Runnable {

        @Override
        public void run() {
            synchronized(queue) {
              if (queue.size()>0) {
                mSeries1.appendData(new DataPoint(counter, queue.poll()), true, 100);
                counter++;
              }
            }
        }
    }          

}
Matt Wolfe
  • 8,924
  • 8
  • 60
  • 77
  • Unfortunately, your answer isn't working at all. First of all it is blocking everything form UI - I can do something without ANR (I'm not sure why) but each press takes a lot of time. Second thing is that the graph is not updated in real time but it looks more like a screenshots of data. – sebap123 May 22 '15 at 22:35
0

Try using an AsyncTask that can be executed from your Fragment or Activity. Here is a link to the Android Docs for AsyncTask

public class SomeAsyncTask extends AsyncTask<Object,Void, Object>{
        @Override
        protected void onPreExecute(){

        }
        @Override
        protected Object doInBackground(Object… params) {
        //make your request for any data here
           return  getData();


        }
        @Override
        protected void onPostExecute(Object object){
        //update your UI elements here
        mSeries1. appendData(object);           
        }
    }
Proxy32
  • 711
  • 5
  • 18