0

Suppose i have a class as follows:

public class vector{
    public vector(double x, double y){
        this.x = x;
        this.y = y;
   }
    public double getDist() {
       return Math.sqrt(x*x + y*y);
   }
}

Does this code operate safely with threads? I am a little confused because I know reads of doubles are not atomic, but because this class has no setters or getters is anything needed to be done in order to ensure this is thread safe?

Thanks

user3358192
  • 1
  • 5
    Objects of this class are immutable. – Sotirios Delimanolis Feb 26 '14 at 23:30
  • 3
    Or is it? What if it has `public double x, y;`? The fields are not shown! – nneonneo Feb 26 '14 at 23:31
  • This object is immutable, immutable objects are automatically thread-safe and have no synchronization issues – betteroutthanin Feb 26 '14 at 23:34
  • 4
    There's not enough information: in fact this isn't even a complete class without the declarations of `x` and `y.` – user207421 Feb 26 '14 at 23:44
  • 4
    Considering that this is perfectly *invalid* Java code I must say, yep it's perfectly thread-safe. Code that doesn't compile generally can't lead to any race conditions or other things at runtime. – Voo Feb 26 '14 at 23:45
  • okay, my apologies, i understand the code isn't complete, but if it was set up very similar to this but complete and two threads tried to getDist() of the same vector object, would it be considered thread safe. – user3358192 Feb 26 '14 at 23:53
  • 1
    @user3358 You are missing the most important information, which makes it pointless to answer the question. Are x and y final, volatile, nothing? This is absolutely essential to the question. – Voo Feb 26 '14 at 23:54
  • it is on a practice exam i have and that is exactly how the question is set up, which is why I am fairy confused about it. – user3358192 Feb 26 '14 at 23:54
  • Well then the answer is, that it's impossible to answer it without more information. You can just list all possible versions and the consequences - Gray explains it very well. – Voo Feb 26 '14 at 23:55
  • Like Voo said, this is completely [thread-safe](http://en.wikipedia.org/wiki/Thread_safety) as code that can't compile cannot manipulate data structures. – MirroredFate Feb 27 '14 at 00:41

1 Answers1

3

I am a little confused because I know reads of doubles are not atomic, but because this class has no setters or getters is anything needed to be done in order to ensure this is thread safe?

Yes, you need to make sure that x and y are marked as final. That has nothing to do with it being double however. final fields are guaranteed to be fully initialized when the vector (it should be Vector btw) is constructed. Without the final if you shared an instance of vector without synchronization, the compiler can reorder the field initializations past the point when the constructor finishes and the x and y fields may be not initialized or partially so. So another thread might see x and y as 0 or with only one of the words updated. For more information, see Constructor synchronization in Java

If you are taking about mutable fields (that can't be final) then whether or not the double will be fully updated depends on the architecture you are running on. To be safe you will have to make them volatile to make sure that they are fully updated and published when they are changed.

As @Voo pointed out, if you are updating x and then y then that is 2 separate operations and your distance calculation might see only one of the two fields updated -- even if they are both volatile. If you need that to be atomic then you should use an AtomicReference and have a little container class that holds both x and y. AtomicReference wraps a volatile Object.

Something like:

 private final AtomicReference<XAndY> xyRef = new AtomicReference<XAndY>();
 ...
 public void setXAndY(double x, double y) {
     xyRef.set(new XAndY(x, y));
 }
 ...
 public double getDist() {
    // get a local instance of our object which is atomic
    XAndY xAndY = xyRef.get();
    return Math.sqrt(xAndY.x * xAndY.x + xAndY.y * xAndY.y);
 }
 ...
 private static class XAndY {
     double x;
     double y;
 }
Community
  • 1
  • 1
Gray
  • 115,027
  • 24
  • 293
  • 354
  • 1
    Even worse: Without the `final` (or `volatile`) modifier, there's no guarantee that the read/write itself will be atomic, which means you could see a value for `this.x` that contains only half the bit pattern of `x` (it is still guaranteed that the rest is 0 at least). – Voo Feb 26 '14 at 23:45
  • 1
    About your second paragraph: To get some kind of valid distance (ignoring that we can't update both variables at the same time which could certianly be considered a race condition) you also have to read x, y into local variables before computing the distance. Otherwise you may read eg x once as 5 and then as 10. – Voo Feb 26 '14 at 23:58
  • 1
    Also note that each variable is being read twice for the multiplications, so just plain volatile wouldn't work either (each read could read a different value of the same variable). – vanza Feb 27 '14 at 00:19
  • @Gray My main point was actually that say we have `x=1, y=0` and T1 does `getDist()` while T2 does `x=2`. I would expect getDist to either return `1` or `2`, but not `sqrt(2)`. Your example code is still not thread-safe this way, because you're not working on a local variable but read the global one repeatedly! – Voo Feb 27 '14 at 00:23
  • 1
    No @Voo. I get a local instance from the reference. – Gray Feb 27 '14 at 00:25
  • @Gray Gosh, sorry. Damn I think that proves that I should go to bed and stop coding ^^ – Voo Feb 27 '14 at 00:26