0

I am using the following code:

public class SomeClass extends Thread {
    ...  
    private double a;
    private double b;

    private double increaseVal;

    public void setIncreaseVal(double increaseVal) {
        this.increaseVal = increaseVal;
    }

    private void updateAB() {
        ...
        // Computing the new a and b values
        a = ... * increaseVal;
        b = ... * increaseVal;
        ...
    }    

    @Override
    public void run() {
        try {
            ...                
            updateAB();
            Thread.sleep(500);
        }
        ...
    }
}

public class OtherClass {
    ...
    private HashMap<String, SomeClass> someClasses;

    private void setIncreaseValue( String id, double value){
        ...
        someClasses.get(id).setIncreaseValue(value);
        ...
    }
}

Is it possible to lock somehow the increaseVal in SomeClass updateAB method such that when it is processed the same increaseVal value is employed when both a and b fields are updated?

With the current code if setIncreaseValue from OtherClass is called it could change the value of increaseVal after a is updated. In this manner the new value of increaseVal will be used when b is updated. I would like to avoid that.

If I put synchronized(this) before updating the a and b value would it work?

Like this:

synchronized(this) {
    // Computing the new a and b values
    a = ... * increaseVal;
    b = ... * increaseVal;
} 

Thanks!

Mihai
  • 189
  • 2
  • 10

3 Answers3

0

You may want to lock increaseVal instead if the increaseVal is your concern. i.e

synchronized(increaseVal) {
    // Computing the new a and b values
    a = ... * increaseVal;
    b = ... * increaseVal;
} 
Anthony
  • 116
  • 8
0

Make the increaseVal as an AtomicInteger. Making it atomic will resolve you issue.

KayV
  • 12,987
  • 11
  • 98
  • 148
0

In order to properly implement thread safe code you have to first identify the state in the class that needs to be made thread safe. In your example the state is represented by three fields

private double a;
private double b;
private double increaseVal;

The next step is to make sure all (i.e. both reads and writes) access to the state is synchronized using the same lock object.

In your example this can be easily implemented using this as the lock and then adding the synchronized keyword to the setIncreaseVal(...) and updateAB() methods:

public synchronized void setIncreaseVal(double increaseVal) {
    ...
}

private synchronized void updateAB() {
    ...
}

This will make your code thread safe: a and b will always be updated using the same increaseVal. However, the code that reads a and b may still read a and b while another thread is running updateAB() and so there is no guarantee that the client code will use a and b as updated by the same increaseVal: if this is what you need, you will have to synchronize also the access to a and b (but the code is not shown in your example, so I can't say much about it).

As regards the synchronization of updateAB(): if there is a lot of code in the method that doesn't use a, b and increaseVal and may take long time to run (e.g. because of I/O operations, like DB access) then it makes sense to synchronize only the code that uses a, b and increaseVal rather than the whole method:

private void updateAB() {
    ...
    synchronized(this) {
        // Computing the new a and b values
        a = ... * increaseVal;
        b = ... * increaseVal;
    }
    ...
}   
  • Thanks a lot for your valuable information Jacopo. If you don't mine, as I am rather new to multithreading, what would other lock than `this` look like? Or how would it be possible to make a specific lock for `a`,`b` and `increaseVal` only? – Mihai Oct 14 '16 at 13:26
  • You are most welcome. Any object can be used as a lock. For example you can define this object as a field of your class as in this way: `private Object aLock = new Object();` and then use it in a synchronized block: `synchronized(aLock) {...}`. Be aware that if you use different locks and the code is not well designed then it may be not thread safe or you could cause deadlocks. – Jacopo Cappellato Oct 14 '16 at 13:37