views:

58

answers:

2

I am curious about the safety of this method I have considered using for passing touch events in my Android application (and testing my understanding of concurrency in Java). Here's the basics:

I have a SurfaceView hooked up to SurfaceHolder.Callback to get user input events, mainly the onTouchEvent callback. After the onTouchEvent method is invoked, I see if event.getAction() == MotionEvent.ACTION_UP and, if so, call a method I have named postTouchEvent which is a member method of my app thread that updates the application's state and draws to the canvas.

SurfaceView

@Override
public boolean onTouchEvent(final MotionEvent event)
{
    mAppThread.postTouchEvent(event);
}

AppThread

private volatile MotionEvent mInputEvent;

public void postTouchEvent(final MotionEvent event)
{
    this.mInputEvent = event;
}

@Override
public void run()
{
    // while loop ...

    if (this.mInputEvent != null)
    {
        final MotionEvent inputEvent = this.mInputEvent;
        this.mInputEvent == null;

        // update state based on inputEvent (not this.mInputEvent)
    }

    // draw to canvas
}

Now I understand that it is certainly not atomic, but since I treat it as immutable after receiving it from framework, won't this work? (Instead of synchronizing the post method and the if statement, which I have no problem doing, but I'm asking to learn.)

Here are my thoughts. I know I will have a valid reference to the object but I am unsure in what state I will actually see the object. While testing everything is working out just fine but I am aware of how uncommon threading exceptions can be, even if something is broken.

Also, I can see one problem with this: If another MotionEvent comes though, it's possible that inputEvent in the run() method will be set to a different event than the one that was referenced when this.mInputEvent != null was checked, but this really isn't an issue.

So, is there anything I am missing or, for my purposes, should this be ok?

+1  A: 

I think that is "safe", modulo the point that you have already identified about lost events.

There is a "happens-before" relationship for a write action on a volatile and a subsequent read action. This, combined with the "happens-before" relationships inherent in actions within a single thread, should mean that your app thread will see the true state of event object as of the time that its reference was written to the volatile.

Of course, if the event listener thread changes the state of the event object after updating the reference, there are no guarantees that the app thread will see them. Ditto, if a third thread updates the event object.

Stephen C
+1  A: 

No, this is not safe, but not for the reason you might expect.

ViewRoot.java, line 1841. This is the code that dispatches your MotionEvents down the view hierarchy. Line 1841 is part of a finally block that calls recycle() on the MotionEvent that has just been dispatched.

MotionEvents are not left to be garbage collected like most objects, they are pooled and recycled to avoid unnecessary memory allocation and garbage collection during event dispatch. Recycling a MotionEvent returns it to the object pool to be used again later when a new MotionEvent is needed. After recycle() has been called, a MotionEvent should be considered invalid.

Your example code may end up reading a MotionEvent object that has been reused by the framework and now contains completely different data.

If you plan to hang on to a MotionEvent after onTouchEvent returns, clone it using MotionEvent.obtain(event). The static obtain() method will return a new MotionEvent from the object pool with the same content.

You should call recycle() when you are done with a MotionEvent that you obtain()ed yourself to return it to the pool. It's not a huge deal if you forget this step, it will become regular Java garbage and the framework will create new MotionEvents when needed. However, MotionEvents can be dispatched very rapidly by the system to the point where cooperating with this optimization can make a nontrivial difference in performance on some devices.

adamp
This is exactly what I was looking to learn. Thank you for taking to time to provide such a detailed answer and suggested solution.
Guzba
You're quite welcome! :)
adamp