CODEWITHSUNDEEP
javasynchronization
L4zl0w
L4zl0w

Reputation: 1097

Synchronizing Java method

Let's say I have the following code running as part of a web service. The code has to be thread safe e.g. no other web service call can change a variable in another instance. Does this code fulfil this requirement?

public class ExampleClass {

   public static String abc = "000";
   public static ArrayList<String> myList = new ArrayList();

   public static synchronized final void clearList(){
   mylist.clear();
   } 
   public static synchronized final void addToList(String listItem){
   myList.add(listItem);
   }

   ...

   more static synchronized methods...

}

I inherited this code and need to make the smallest amount of changes to ensure it's synchronized.

Many thanks for your answers.

[edited] To give some clarity. A web service call comes in executing this code and it's working with 'abc' and 'myList' variables e.g. changing them. In the meantime another web service call comes in and it will start working with these variables as well. However these two separate requests should have their own 'abc' and 'myList' variables e.g. they can't share these between them, otherwise the result will be wrong.

Upvotes: 0

Views: 264

Answers (4)

fdreger
fdreger

Reputation: 12505

[edit: an addendum]

Thread safety is just an expressions, not an absolute term. Unless you define what you mean, it's hard to help.

For example: you don't describe how "abc" is accessed and what are your expectations. If you, for example, require that changes to abc and the list are atomic, then you need to wrap the code that accesses them into a single synchronized block. If "abc" never changes, then you don't need to add anything to make it thread-safe. If it does, you need to wrap all the modifications and reads into a synchronized block (or you are going to have a nasty visibility problem). And if there is only one thread that makes changes to abc, then you should be fine with making it volatile.

The two problems are:

  • myList is public, so it is hard to make any guarantee or even assumtpions about the code using it. If you make it private, you will at least only have to worry about your own methods. You might want to skip synchronization of the methods and go straight to: myList = Collections.synchronizedList(new ArrayList()); this way all the methods will be synchronized on myList object; if you decide to do so, you will have to make myList final (so that noone can switch it for an unsynchronized version);
  • your description: "no other web service call can change a variable in another instance" has nothing to do with being thread-safe. Anyone can change anything in myList, since you provide convenient means of doing so (public static methods). Also, all instances of ExampleClass share myList, and they modify the single instance. The point is that the changes will be thread-safe.

Upvotes: 2

FordFulkerson
FordFulkerson

Reputation: 140

Use the synchronized collections if you can.

Upvotes: 0

Michael Lloyd Lee mlk
Michael Lloyd Lee mlk

Reputation: 14661

The variables are public so no. Make them private. If you have any methods that return the list object they will need to be changed to defensively copy the list.

Upvotes: 1

Reimeus
Reimeus

Reputation: 159864

No. Your String abc and ArrayList myList can clearly be accessed by another class. Make them private (and preferably non-static).

Upvotes: 3

Related Questions