TheClassic
TheClassic

Reputation: 1044

Java thread safety of setting a reference to an object

I'm wondering if the following class is thread safe:

class Example {

private Thing thing;

public setThing(Thing thing) {
    this.thing = thing;
    }

public use() {
    thing.function();
    }
}

Specifically, what happens if one thread calls setThing while another thread is in Thing::function via Example::use?

For example:

Example example = new Example();
example.setThing(new Thing());

createThread(example);  // create first thread
createThread(example);  // create second thread

//Thread1
while(1) {
  example.use();
}

//Thread2
while(1) {
    sleep(3600000); //yes, i know to use a scheduled thread executor
    setThing(new Thing());
}

Specifically, I want to know, when setThing is called while use() is executing, will it continue with the old object successfully, or could updating the reference to the object somehow cause a problem.

Upvotes: 1

Views: 646

Answers (2)

Mikita Harbacheuski
Mikita Harbacheuski

Reputation: 2253

There are 2 points when reasoning about thread safety of a particulcar class :

  1. Visibility of shared state between threads.
  2. Safety (preserving class invariants) when class object is used by multiple threads through class methods.

Shared state of Example class consists only from one Thing object.

  1. The class isn't thread safe from visibility perspective. Result of setThing by one thread isn't seen by other threads so they can work with stale data. NPE is also acceptable cause initial value of thing during class initialization is null.
  2. It's not possible to say whether it's safe to access Thing class through use method without its source code. However Example invokes use method without any synchronization so it should be, otherwise Example isn't thread safe.

As a result Example isn't thread safe. To fix point 1 you can either add volatile to thing field if you really need setter or mark it as final and initialize in constructor. The easiest way to ensure that 2 is met is to mark use as synchronized. If you mark setThing with synchronized as well you don't need volatile anymore. However there lots of other sophisticated techniques to meet point 2. This great book describes everything written here in more detail.

Upvotes: 1

CMPS
CMPS

Reputation: 7769

If the method is sharing resources and the thread is not synchronized, then the they will collide and several scenarios can occur including overwriting data computed by another thread and stored in a shared variable.

If the method has only local variables, then you can use the method by mutliple threads without worring about racing. However, usually non-helper classes manipulate member variables in their methods, therefore it's recommended to make methods synchronized or if you know exactly where the problem might occur, then lock (also called synchronize) a subscope of a method with a final lock/object.

Upvotes: 1

Related Questions