1

I have the following class:

public class TextSuggester {

    private Suggester suggester;

    public TextSuggester() {
        createSuggester();

    public void refresh() {
        createSuggester();
    }

    private void createSuggester() {
        suggester = new Suggester(File file); // expensive operation
    }    

    public String lookup(String text) {
        return suggester.lookup(text);
    }

}

The lookup method will be accessed from multiple threads. The refresh method will be called every hour and is expensive (takes a long time). My questions are:

1) Is this design thread-safe?

2) If the lookup method is called by another thread after refresh is called, but before refresh returns, will it use the "old" suggester object for the lookup?

If the answer is no to either question, then how do I accomplish what I need? In essence, I want the "old" suggester to be used while the new one is being created.

stepanian
  • 11,373
  • 8
  • 43
  • 63
  • Nothing is thread safe right now. Make your field `volatile` and perform the new assignment lazily. – Sotirios Delimanolis Nov 05 '15 at 00:37
  • @SotiriosDelimanolis Thanks for the quick response. Could you elaborate or post an answer? – stepanian Nov 05 '15 at 00:38
  • 1
    Making your `suggester` field `volatile` will ensure that any thread reading the field will see the latest value written to it by another thread. If a new `Suggester` is being created when `lookup` gets called, then the calling thread will use the most recently written one (the `Suggester` created on the last `refresh` call). – Evan LaHurd Nov 05 '15 at 00:45

1 Answers1

1

Your current code is not thread safe. Nothing guarantees that the value of suggester seen by thread A within lookup will be the same as the value of suggester assigned by thread B in createSuggester.

What you need to do is add some kind of synchronization so that all changes are visible. Since all your methods are basically just interacting with suggester, your happens before ordering should be relative to that field. volatile is an option. AtomicReference is another.

So either

private volatile Suggester suggester; // with standard field access and assignment

or

private final AtomicReference<Suggester> suggesterRef; // initialized correctly in constructor
...
private void createSuggester() {
    suggesterRef.set(new Suggester(File file));
}   

until set is actually invoked, all other threads calling get (in lookup) will see the previously assigned value.

Sotirios Delimanolis
  • 274,122
  • 60
  • 696
  • 724
  • You also mentioned lazy loading in your comment to be used with "volatile". Is that also necessary? Or just making Suggester be volatile is enough? – stepanian Nov 05 '15 at 00:55
  • @stepanian The lazy loading refers to the fact that the variable or `AtomicReference` will only be set when the new `Suggester` object is fully initialized. – Sotirios Delimanolis Nov 05 '15 at 00:58
  • Got it. So the "volatile" keyword is what is accomplishing the lazy loading, right? Also, just for curiosity, what happens if "volatile" is not used and the thread tries to access "suggester" BEFORE the other thread completes? If it's not the old object, then what? Does it crash? – stepanian Nov 05 '15 at 01:00
  • @stepanian The assignment to it does. Your `TextSuggester` is itself acting like a reference to a `TextSuggester`. If `volatile` isn't used, the JVM won't crash. But it's allowed to return the old value or the new value. This is part of the Memory Model of Java and without using `volatile`, there are many possible program orderings. You can read more about `volatile`, [here](http://stackoverflow.com/questions/106591/do-you-ever-use-the-volatile-keyword-in-java). – Sotirios Delimanolis Nov 05 '15 at 01:05
  • Sorry, one last question: If the threads are very short-lived and there is no requirement for them to access the latest suggester object, then would I still need to use "volatile"? Will threads created later eventually access the more recent suggester objects? Thanks. – stepanian Nov 05 '15 at 02:29
  • @stepanian That's not an easy questions to answer. It depends how you publish the `TextSuggester` instance. _Will threads created later eventually access the more recent suggester objects?_ Yes. Calling `start` on a `Thread` (and thereby creating a thread) [_happens-before any actions in the started thread_](https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4.5) – Sotirios Delimanolis Nov 05 '15 at 03:22