CODEWITHSUNDEEP
javasynchronization
gronostaj
gronostaj

Reputation: 2282

Synchronized method seems interrupted

I'm writing an applet with multiple threads. I've had some strange problems an I've tracked them down to the class below. It's the entire code, no snips.

public class BoundingBox {
    private volatile int top;
    private volatile int left;
    private volatile int bottom;
    private volatile int right;

    public static final int IGNORE  = 0;
    public static final int TOP     = -2;
    public static final int LEFT    = -1;
    public static final int BOTTOM  = 2;
    public static final int RIGHT   = 1;

    BoundingBox(int left, int top, int width, int height) {
        this.top = top;
        this.left = left;
        this.right = left + width;
        this.bottom = top + height;
    }

    public synchronized int top()       { return top;           }
    public synchronized int left()      { return left;          }
    public synchronized int bottom()    { return bottom;        }
    public synchronized int right()     { return right;         }

    public synchronized int width()     { return right - left;  }
    public synchronized int height()    { return bottom - top;  }

    public synchronized void translate(Vector2D vector) {
        left += vector.getX();
        right += vector.getX();
        top += vector.getY();
        bottom += vector.getY();
    }

    public synchronized void alignTo(Point2D point, int hAlign, int vAlign) {
        if ((hAlign != IGNORE && hAlign != LEFT && hAlign != RIGHT)
            || (vAlign != IGNORE && vAlign != TOP && vAlign != BOTTOM))
            throw new IllegalArgumentException();

        /// START DEBUG CODE ///
        if (right - left != width())
            System.out.println("X");
        if (bottom - top != height())
            System.out.println("Y");
        /// END DEBUG CODE ///

        int width = width();
        int height = height();

        if (hAlign != IGNORE) {
            left = point.getX();
            if (hAlign == RIGHT)
                left -= width;
            right = left + width;
        }

        if (vAlign != IGNORE) {
            top = point.getY();
            if (vAlign == BOTTOM)
                top -= height;
            bottom = top + height;
        }
    }
}

X and Y do print sometimes. As you can see, width() is defined as right - left, but it still happens that these two aren't equal (same with height()). Those four fields are private and methods are synchronized, so nothing should interrupt alignTo, right? Still, something does as it seems to me.

What's wrong with this code?

Upvotes: 0

Views: 92

Answers (1)

Fried Sock
Fried Sock

Reputation: 128

Declaring top, left, bottom, right as volatile is not enough to keep them synchronized in the way that you need. The problem is, those variables are being modified on different threads using the translate method, so are changing during the execution of alignTo. You will need to put a lock on those variables for the duration of alignTo, or cache them to local variables.

Upvotes: 1

Related Questions