0

Where do I need to place the keyword synchronized if I want to prevent that the two threads are manipulating tab simultaneously?

Mainclass owns the variable tab it's methods f1,f2,f3 are manipulating tab. f2,f2 are called by the main method in a loop, and f3 is called by a thread, in x milliseconds after a event occurs in the main method.

main method:

main() {
    Mainclass mainclass = new Mainclass(); // class containing variable tab
    while (condition) {
        mainclass.f1(); // manipulating tab
        mainclass.f2(); // manipulating tab
        if (eventOccured) {
            mainclass.startThread(x); 
            // thread calls f3() after x milliseconds whitch is manipulating tab
        }
    }
}

IMainclass:

public class Mainclass {
    public void f1(){...}  // manipulating tab
    public void f2(){...}  // manipulating tab
    public void f3(){...}  // manipulating tab
    public final TabObject[][] tab;
}

Can I synchronize tab, or do I have to synchronize f1,f2,f3, should I use a synchronized block like synchronized (tab) {...} or syncronize the whole mehtods?

Armin
  • 351
  • 1
  • 4
  • 29

3 Answers3

2

The simplest would be to synchronize f1, f2 and f3. However, this will imply that at any moment, at most one of these methods can run. This can have some performance issues if those methods are performing some heavy computations not related to tab.

For example, assume the following :

public void f1() {

    /*
       Do some I/O, e.g. read in/write very large files (typically very slow)
    */

    synchronized(tab) {
        // Than manipulate "tab"
    }
}

In this case it is clearly better to have a synchronized block around the tab manipulations instead of synchronizing the whole method. Doing so, f1, f2 and f3 can run at the same time to do some slow I/O operations and only synchronize upon manipulating tab, possibly resulting in a better performance.

Kevin
  • 2,813
  • 3
  • 20
  • 30
  • @svasa Found this topic, pretty well explaining why you should not do that. http://stackoverflow.com/questions/10195054/synchronized-object-set-to-null – Kevin Feb 14 '16 at 15:15
  • @svasa Note that in this case `tab` is declared as `final` so one won't be able to set it to null afterwards. – Kevin Feb 14 '16 at 15:21
  • I know why I should not do that, but if I use some getter method to reset my "tab" like tab = getValues(); This could well be from a third party jar and for some reason getValues() returned null. Before that, if tab is declared final - how can it be "manipulated"? – SomeDude Feb 14 '16 at 15:29
  • Knowing that it could possibly return `null` you should explicitly have checked it before assigning it. Ps: if you do `tab = ...` it's not a getter but a setter for tab. – Kevin Feb 14 '16 at 15:31
  • Ok. I mean setting tab value using a getter. – SomeDude Feb 14 '16 at 15:36
  • @svasa i don't think it's a good practice to have a getter with side effects, feel free to correct me if i'm wrong. – Kevin Feb 14 '16 at 15:40
  • You can manipulate the content, but not the variable itself, if it is declared as final. You can't change the pointer (`tab = new ...()` or `= null`), but you can do sth like that: `tab[1][0] = 5` – Armin Feb 14 '16 at 15:40
2
synchronized void A(){
    ...
}

is equivalent to

void A(){
    synchronized(this) {
        ...
    }
}

but using this as lock is not a good idea.

Without knowing the details of the methods, you can just use the synchronized (tab) {...} block as you said, but I'd use it only to wrap lines that are actually reading/writing the tab var instead of having it for the whole method.

If you want to make things cleaner you can even add

private final Object lock = new Object();

and use synchronized (lock) {...} instead on synchronizing to tab (which is public, meaning that another class may synchronize to it leading to deadlock).

Community
  • 1
  • 1
StepTNT
  • 3,867
  • 7
  • 41
  • 82
  • The Object `lock` would just be as marking that a critical operation is running right now, or? – Armin Feb 14 '16 at 15:22
  • 1
    In a sense. Basically, each *Java* object has an implicit lock used for synchronization (that's why a `synchronized` method is like a `synchronized(this)` block), but using a `private Object` instead of `this` allows your code to be more "safe". I'm not really sure if that's what you asked, so if I misunderstood your question please let me know :) – StepTNT Feb 14 '16 at 15:25
1

You can define a monitor object like private Object monitor = new Object(); and call the code manipulating tab inside a synchronized block like

You can place this synchronized block inside your methods f1,f2,f3 like :

public void f1()
{

   synchronized(monitor)
   { 
     //code to manipulate tab
   }
}
SomeDude
  • 13,876
  • 5
  • 21
  • 44