Can this "synchronized" work?

1. Is the way to use synchronized here correct?

2. Do the randomScore is locked when one thread access to it so that other threads can not access to randomScore?开发者_如何学Python

3. If only changeRandomScore() can access to randomScore variable and only one thread can access to changeRandomScore(), therefore only one thread can acccess to randomScore at a time. Is it correct?

    import java.*;


public class StudentThread extends Thread {
    int ID;  
    public static int randomScore;

      StudentThread(int i) {
          ID = i;
      }

      public void run() {
          changeRandomScore();

          System.out.println("in run");
      }
public synchronized void changeRandomScore() {
    randomScore = (int) (Math.random()*1000);
}
public static void main(String args[]) throws Exception {
    for (int i = 1;i< 10 ;i++) 
    {
            StudentThread student = new StudentThread(5);

            student.start(); 
            Thread.sleep(100);
            System.out.println(randomScore);
    }             
}  
}

You are accessing here a static variable inside of synchronized-methods of different objects. The synchronization has no real effect here, since each thread uses its own monitor (the thread object, in this case). For a shared variable you should use a shared monitor, too.

Here is a variant that is synchronized correctly:

public class StudentThread extends Thread {
  int ID;  
  private static int randomScore;
  private static final Object scoreLock = new Object();

  StudentThread(int i) {
     ID = i;
  }

  public void run() {
     changeRandomScore();

     System.out.println("in run");
  }
  public void changeRandomScore() {
     int tmp = (int) (Math.random()*1000);
     // no need to synchronize the random()-call, too.
     synchronized(scoreLock) {
        randomScore = tmp;
     }
  }
  public static void main(String args[]) throws Exception {
      for (int i = 1;i< 10 ;i++) 
      {
          StudentThread student = new StudentThread(5);

          student.start(); 
          Thread.sleep(100);
          synchronized(scoreLock) {
              System.out.println(randomScore);
          }
      }             
  }  
}

The difference is that we are now using a common lock object (scoreLock) and use this as parameter for synchronized-blocks, and we also synchronize on this object in the main method on reading the score.

Alternatively we could also declare the method public static synchronized void changeRandomScore() (which means it uses the class-object as the monitor) and in the main-method synchronize on StudentThread.class.

Yeah, as others said: if you want to ensure that a variable is accessed only with proper synchronization, don't make it public.

Correct, only one thread will be able to access the method changeRandomScore() at a time. However, that doesn't mean that multiple threads won't be able to modify the variable randomScore concurrently. It only guarantees that only one thread can execute that method at a time. That is especially true given that randomScore is public.

Edit Allow me to rephrase and take a stab at answering your "bigger question" rather than the explicit question you posed in your post. In order to guarantee synchronization and that only one thread can access randomScore at once, you must implement some sort of locking mechanism that controls all reads and writes of the variable. In order to successfully accomplish that, you must define an interface that controls access to the variable. You haven't done that in your example. You need to restrict the access to the variable by using private and then create methods that allow manipulation of the variable. You can accomplish this through method synchronization, but that may not always be the correct tool for the job.

For example this:

public void synchronized myMethod(){
  //code
}

Is equivalent to

public void myMethod(){
  synchronized(this){
    //same code
  }
 }

So what you're doing with method-level synchronization is synchronizing on the instance of the object. This is fine if you're only manipulating data members internal to the object, but this falls apart if the variables you are working with have any sort of scope outside of the object (for instance, in your case a static variable). That case, you would need some sort of other locking mechanism.

No, you don't use it right. The writing is synchronized, that's fine, but buys you nothing at all. Without the synchronizations, it'd work the same, because

• access to int is atomic anyway
• reading is not synchronized, so you may see stale values

You'd better never expose field needing synchronization.