CODEWITHSUNDEEP
javalinked-listsynchronized
user3302074
user3302074

Reputation:

synchronized LinkedList in Java

when I have two LinkedLists, product and price declared on the top of a class. And there are some threads who can write to this lists, I must(!) safe this Lists with a Mutex (synchronized). Is the following correct and is there a better way?

public class Shop extends Thread {
  volatile LinkedList<String> product = new LinkedList<String>();
  volatile LinkedList<Float> price = new LinkedList<Integer>();

  public class Inserter extends Thread {
   @Override
   public void run() {
    synchronized (product) {
     product.add(i1);
    }

    synchronized (price) {
     price.add(i2);
    }
   }
  }
}

Upvotes: 0

Views: 1712

Answers (3)

AlexR
AlexR

Reputation: 115378

No, your code is incorrect. You are using separate synchronized blocks for update of your separate collections. This means that you can fail on data incosistancy when new product has been already added by price has not be attached yet. So, if other thread does something like this:

int n = product.size() -1;
Product prod = product.get(n); //OK
Price pr = price.get(n); // throws exception

exception will be thrown.

Better solution is to synchronized both operation using 1 lock:

Object productPriceLock = new Object(); 
.............

public void run() {
    synchronized (productPriceLock) {
        product.add(i1);
        price.add(i2);
    }
}

This code is better but a little bit ugly. Better solution is not to hold parallel colecitons but create special class that holds both product and price:

class Deal {
    private final Product product;
    private final Price price;
    public Deal(Product product, Price price) { ... }
    .......
}

Please pay attention that the class Deal is immutable.

Now you can put it into one collection and if you want synchronize it. But better use one of available concurrent collections. It is much more efficient and less error prone.

Upvotes: 0

tom87416
tom87416

Reputation: 542

I have few comments

  1. I don't see the meaning of volatile... It is a Reference type pointing to the same list all the time, even if you add/remove elements from the list. (This is a mutable list)
  2. When 1 threads want to add (product1, price1) and another thread add (product2, price2), the order of both list may out of order. e.g
    Product = {product1, product2} and Price = {price 2, price1}. You can add a sleep in one thread and you will see.

code in this is better

synchronized (product) {
     product.add(i1);
     synchronized (price) {
         price.add(i2);
    }
}

2 idea

Synchronized the Inserter.class

synchronized(Inserter.class){
   product.add(i1)
   price.add(i2)
}

Synchronized on a global object. Apply to the case which Inserter is not the only class to do the manipulation.

Object objLock = new Object()
...
synchronized(objLock){
   product.add(i1)
   price.add(i2)
}

Upvotes: 0

Luiggi Mendoza
Luiggi Mendoza

Reputation: 85779

Decorate your LinkedList using Collections#synchronizedList instead:

volatile List<String> product = Collections.synchronizedList(new LinkedList<String>());

Upvotes: 2

Related Questions