Java is this class threadsafe

Question

Just for me I wrote class for Google Maps v2 (Android) to organize a queue of move/zoom actions. If you know animate action is cancelled if another animation has started while first animation is still running.

So to avoid this issue I wrote class. It's working but I want to know is it thread safe? Maybe I missed something? Thanks in advance.

public class MapDecoratorQueue {

    private List<MapDecorator> queue = new Vector<MapDecorator>();
    private boolean isRunning = false;

    public synchronized void add(MapDecorator md) {
        if (isRunning) {
            queue.add(md);
        } else {
            queue.clear();
            queue.add(md);
            next(0);
            isRunning = true;
        }
    }

    private void next(final int pos) {
        if (queue.isEmpty()) {
            isRunning = false;
        } else {
            MapDecorator decorator = queue.get(pos);
            L.log(MapDecoratorQueue.class.getSimpleName(), "isMove: "
                    + decorator.isMove() + " isZoom: " + decorator.isZoom());
            if (decorator.isMove()) {
                decorator.map.moveCamera(decorator.move);
            }
            if (decorator.isZoom()) {
                decorator.map.animateCamera(decorator.zoom,
                        decorator.zoomDuration, new CancelableCallback() {

                            @Override
                            public void onFinish() {
                                queue.remove(pos);
                                next(pos);
                            }

                            @Override
                            public void onCancel() {
                                L.log(MapDecoratorQueue.class.getSimpleName(),
                                        "cancelled");
                                queue.clear();
                                isRunning = false;
                            }
                        });
            }
        }
    }

    public static class MapDecorator {
        private GoogleMap map = null;
        private CameraUpdate move = null;
        private CameraUpdate zoom = null;
        private int zoomDuration = 0;

        /**
         * Для передвижения камеры и анимации
         * 
         * @param move
         * @param zoom
         * @param zoomDuration
         */
        public MapDecorator(GoogleMap map, CameraUpdate move,
                CameraUpdate zoom, int zoomDuration) {
            this.map = map;
            this.move = move;
            this.zoom = zoom;
            this.zoomDuration = zoomDuration;
        }

        /**
         * Только для передвижения камеры
         * 
         * @param move
         */
        public MapDecorator(GoogleMap map, CameraUpdate move) {
            this(map, move, null, 0);
        }

        /**
         * Только для зума камеры
         * 
         * @param zoom
         * @param zoomDuration
         */
        public MapDecorator(GoogleMap map, CameraUpdate zoom, int zoomDuration) {
            this(map, null, zoom, zoomDuration);
        }

        public boolean isMove() {
            return map != null && move != null;
        }

        public boolean isZoom() {
            return map != null & zoom != null && zoomDuration >= 0;
        }

        public CameraUpdate getMove() {
            return move;
        }

        public CameraUpdate getZoom() {
            return zoom;
        }

        public int getZoomDuration() {
            return zoomDuration;
        }
    }

}

Answer 1

If in line below CancelableCallback is used asynchronously then you are in trouble as you are leaking access to next(pos) method to other threads and in that case MapDecoratorQueue is not ThreadSafe.

decorator.map.animateCamera(decorator.zoom,decorator.zoomDuration, new CancelableCallback(pos));

Anyway as per Brian Goetz :), thread safety should not depend on clients implementaion (in this case animatecamera) so Your class is not threadsafe.To make it thread safe you can make next synchronized and if you do so you dont need Vector. ArrayList will work fine.

Answer 2

The list (Vector in your cas) is thread safe but the way isRunning was used is not. You can have a race condition when a Thread tries do get an element with next method and when another tries to add one with add. If first thread is just before setting isRunning to false and the second checks it at that point you have a problem.

Adding synchronized to next method will solve the issue.