Question

Are there any synchronizing/reference issues with this code?

(Assume that myStrings is already instantiated.)

MySynch.java:

public class MySynch
{
    public static String[] myStrings = new String[10];

    public static void updateStrings()
    {
        synchronized (myStrings)
        {
            myStrings = new String[10]; // Safe?

            myStrings[0] = something[0];
            myStrings[1] = somethingElse[4];
        }
    }
}

The object array myStrings may be read from by more than one thread, and has one thread that updates (writes) it by running updateStrings(). The threads that read from it will also use a synchronized (myStrings) block to read from it, of course, for safety.

Are there issues with locking the array and instantiating it again inside the synchronized block which is locking it (as above)?

Was it helpful?

Solution

There is an synchronization issue: when myStrings is set to a new instance, and a second thread is executing the method just after, that second thread will synchronize the second instance of myStrings.

You should synchronized on the class or any other static final object with

synchronized(MySynch.class) {
    ...
}

OTHER TIPS

The only problem that I can see is there's a possibility that a read could be performed before the array has been properly instantiated.

You will get inconsistent value for myStrings, better have two synchronized blocks or methods one for update and another one for get on a class lock and have your myStrings private.

// update
synchronized (YourClass.class) {
    // update
}

// get
synchronized (YourClass.class) {
    // get
}

Cleaner, IMO.

public class MySynch {
private static AtomicReference<String[]> myStrings = new AtomicReference<String[]>(
        new String[0]);

public static void updateStrings() {
    String[] tmp = new String[2];
            ....
    myStrings.set(tmp);

}

public static String[] getStrings() {
    return myStrings.get();
}

}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top