Search code examples
javaandroidsynchronizedsynchronized-block

How to correctly use synchronized?


This piece of code:

synchronized (mList) {
    if (mList.size() != 0) {
        int s = mList.size() - 1;
        for (int i = s; i > 0; i -= OFFSET) {
            mList.get(i).doDraw(canv);
        }
        getHead().drawHead(canv);
    }
}

Randomly throws AIOOBEs. From what I've read, the synchronized should prevent that, so what am I doing wrong?

Edits:

AIOOBE = Array Index Out Of Bounds Exception The code's incomplete, cut down to what is needed. But to make you happy, OFFSET is 4, and just imagine that there is a for-loop adding a bit of data at the beginning. And a second thread reading and / or modifying the list.

Edit 2:

I've noticed it happens when the list is being drawn and the current game ends. The draw-thread hasn't drawn all elements when the list is emptied. Is there a way of telling the game to wait with emtying the list untill it's empty?

Edit 3:

I've just noticed that I'm not sure if this is a multi-threading problem. Seems I only have 2 threads, one for calculating and drawing and one for user input.. Gonna have to look into this a bit more than I thought.


Solution

  • What you're doing looks right... but that's all:

    1. It doesn't matter on what object you synchronize, it needn't be the list itself.
    2. What does matter is if all threads always synchronize on the same object, when accessing a shared resource.
    3. Any access to SWING (or another graphic library) must happen in the AWT-Thread.

    To your edit:

    I've noticed it happens when the list is being drawn and the current game ends. The draw-thread hasn't drawn all elements when the list is emptied. Is there a way of telling the game to wait with emtying the list untill it's empty?

    I think you mean "...wait with emptying the list until the drawing has completed." Just synchronize the code doing it on the same lock (i.e., the list itself in your case).

    Again: Any access to a shared resource must be protected somehow. It seems like you're using synchronized just here and not where you're emptying the list.