Synchronizing to an object to be instantiated

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)?

score 4 · Accepted Answer · answered Sep 22 '11 at 15:25

4

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) {
    ...
}

answered Sep 22 '11 at 15:25

Laurent Legrand

1,104
6
6

Thanks, that sounds good. Is there anything else I need to do for that to work, like declare the class as a final class (if that can be done), or anything? – Doddy Sep 22 '11 at 15:39
No. There is nothing to do on the class to use it as a synchronization monitor. – Laurent Legrand Sep 22 '11 at 15:50
Note that you can declare a class final. But it means that it cannot be extended. – Laurent Legrand Sep 22 '11 at 15:51

score 2 · Answer 2 · answered Sep 22 '11 at 15:13

2

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.

answered Sep 22 '11 at 15:13

mrkhrts

829
7
17

Oh, just assume that `myStrings` has already been instantiated at the start of the program. – Doddy Sep 22 '11 at 15:16
@panic, Is there a reason you're using the `static` modifier? – mrkhrts Sep 22 '11 at 15:17
Yes, I wont be making instances of the class itself, it is basically a library of common methods and so on. – Doddy Sep 22 '11 at 15:21
1

@panic, Usually utility classes are stateless. Perhaps you ought to make it a singleton instead? – mrkhrts Sep 22 '11 at 15:22
I edited the program to guard against the possibility that a read could be performed before the array has been properly instantiated, but there is nothing to stop some other code from setting the array to null. – emory Sep 22 '11 at 15:25

Vishal · Answer 3 · 2011-09-22T15:36:19.803

2

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
}

edited Sep 22 '11 at 15:36

answered Sep 22 '11 at 15:30

Vishal

19,879
23
80
93

score 1 · Answer 4 · answered Sep 22 '11 at 15:39

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();
}

}

Synchronizing to an object to be instantiated

4 Answers4