Java Concurrent Exception

Question

Hi I have a paint method that is drawing an image and I have another method that is constantly modifying the image to be drawn however I experience concurrency exceptions now and again. What is the most efficient way to resolve this please? I know I could use synchronized blocks on the buffered image but then it throws up warnings on synchronizing a none final variable.

private BufferedImage img;

public void modImage(BufferedImage image) { 
     img = image;
}

public void paintComponent(Graphics g) {
    if (img != null) {
        g.drawImage(img, 0, 0, this);
    }
}

You could alternate between two different buffers. One for rendering and one for editing. — Harvtronix, Apr 29 '15 at 23:56
Sorry could you give an example of how to achieve this using buffers — haz hazzz, Apr 29 '15 at 23:58
There's nothing inherently wrong with synchronizing on a non-final variable. You might have your warning level set unnecessarily high. — user253751, Apr 30 '15 at 00:28
@immibis: There's plenty wrong with it if it's `synchronized (img)`, which is what "use synchronized blocks on the buffered image" sounds like. — user2357112, Apr 30 '15 at 00:35
@user2357112 Sure, there are potential bugs that involve `synchronized` and non-final variables, but IMO the signal-to-noise ratio of that warning is way too low. (Why not a "calling method on non-final variable" warning?) — user253751, Apr 30 '15 at 00:36
How is `modImage` being called? Are you reusing the same `BufferedImage` instance each time? or modifying the image after calling `modImage`. — fgb, Apr 30 '15 at 00:38
Synchronization on img witch creates the warning is no good idea since synchronization on a changing monitors lead to the same unexpected behaviour as the data race it is meant to prevent. By the way synchronization on null will lead to an exception. So the best is to sync on instance as in one of the answers below — Thomas Krieger, May 02 '15 at 19:19

pathfinderelite · Answer 1 · 2015-04-30T00:48:57.493

You have a race condition because the value of img can changed in another thread between the if conditional and the use of img in drawImage

if (img != null) {                     // <-- img can be non-null here
    g.drawImage(img, 0, 0, this);      // <-- img can now be null
}

You can assign img to a local variable, then use the local variable in-place of img in the above code. The local variable will remain constant even if img is changed to a different object in another thread

final BufferedImage localImg = img;      // <-- localImg won't change locally
if (localImg != null) {
    g.drawImage(localImg, 0, 0, this);
}

In addition to this, img should be declared as volatile so its value is not cached thread-locally; changes in one thread will be seen by others.

private volatile BufferedImage img;

Bear in mind that declaring a variable as volatile will cause synchronization to occur whenever that variable is accessed; so synchronization is still happening. However, synchronization occurs on the img reference itself, not the BufferedImage object to which it refers, so there are no issues here if img is null.

Without synchronization, there's no guarantee the thread reading `img` will see the other thread's writes. — user2357112, Apr 30 '15 at 00:32
@user2357112 Good point! `img` would need to be declared as volatile. I will update my answer. — pathfinderelite, Apr 30 '15 at 00:34

score 1 · Answer 2 · answered Apr 30 '15 at 00:39

you could sync on instance,

private BufferedImage img;

public void modImage(BufferedImage image) { 
     synchronized(this){
         img = image;
     }
}

public void paintComponent(Graphics g) {
    synchronized(this){
    if (img != null) {
        g.drawImage(img, 0, 0, this);
       }
    }
}

score 0 · Answer 3 · answered Apr 30 '15 at 01:17

There's a thread-safety issue because the contents of the BufferedImage can be changed while the paintComponent method is running. Synchronization and volatile in the ui class alone won't fix this.

If you want to add synchronization, then you'll need to make sure any modification of the image is synchronized as well. This needs to be synchronized on some object that's shared between each class involved.

You can avoid some synchronization by making sure to set img to a copy of the BufferedImage or a different instance each time. It should still be volatile though, and check pathfinderelite's answer if you want to safely set img back to null.

Java Concurrent Exception

3 Answers3