HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Accessing a String value from multiple threads without synchronization

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withoutsynchronizationthreadsvaluemultiplefromstringaccessing

Problem

I'm looking for some input here. I have a class that contains a value which is updated every few seconds by a method within that class. Right now, access to this value across multiple threads is done via synchronization, which I would like to eliminate. Would this make sense?

class DataSegment {

    private MetricsUpdater metrics = new MetricsUpdater.getInstance();

    public String printValues() {
        StringBuilder sb = new StringBuilder();
        sb.append(value1);
        sb.append(morevalues);
        sb.append(metrics.myValue); // this is the value that's currently synchronized
        return sb.toString();
    }
}

class MetricsUpdater {

    private String myValueSynchronized;
    public String myValue;

    public static MetricsUpdater getInstance() {
        if (theInstance == null) {
            theInstance = new MetricsUpdater();
        }
        return theInstance;
    }

    // this runs on a timer but to keep it simple I'll just define the method...
    private void updateMetrics() {

        synchronized(myValue) {
            // also, to keep things simple, I've replaced all of the actual logic with a method called someMethodToUpdateMyValue()
            myValueSynchronized = someMethodToUpdateMyValue();
            myValue = myValueSynchronized;
        }
    }
}


There can be many instances of DataSegment all reading from myValue, but the metrics class is a singleton. myValue only updates every 5 seconds or so and only MetricsUpdater is allowed to write to it. Does that make sense?

Does it even need to be synchronized at all if all of the other threads are only allowed to read it?

Solution

Your code is not synchronized at all at the moment. There are two major mistakes:

-
You are only synchronizing in the thread(s) which are writing the value, but are never synchronizing on the threads reading the value. You must always synchronized on all threads that read/write a value which is shared between thread. Absolutely nothing is synchronized if you only synchronize on read or only on write.

-
You use synchronize(myValue), but you keep changing myValue, so you are synchronizing on different objects over time, which basically means no synchronization.

By the way, it is possible that you made some tests and everything seemed synchronized. However, you should be aware that those tests would fail on other cpu's, or could fail on your machine if you tried them enough time.

More general comments:

-
Your whole class MetricsUpdater can be dropped for a volatile String myValueSynchronized. Note however that this only work with references to immutable Objects (such as String), or raw types.

-
To answer your question "Does it even need to be synchronized at all if all of the other threads are only allowed to read it?"

NO!

You could forgo synchronization however if the value was final and unmodifiable.

Context

StackExchange Code Review Q#57408, answer score: 3

Revisions (0)

No revisions yet.