Ram
Ram

Reputation: 3

Synchronized ArrayList return non empty list

The below class contains the synchronized arraylist and two methods one to clear and add the arrayLIst and other to get the list.

public class Singleton {
    private static Singleton singleton;
    private List<String> strList = Collections.synchronizedList(new ArrayList<String>());

    private Singleton() {
        
    }
    
    public static synchronized Singleton getInstance() {
        if (singleton == null) {
            singleton = new Singleton();
        }
        return singleton;
    }
    
    public void addElement(List<String> list) {
        strList.clear();
        strList.addAll(list);
        
    }
    
    public List<String> getStrList(){
        return new ArrayList<>(strList);
    }
    
}

When addElement and getStrList are called in two different threads, getting empty list some time.

public class App
{
    public static void main(String[] args) throws InterruptedException
    {
        final Singleton singleton = Singleton.getInstance();

        Thread t1 = new Thread(new Runnable() {
            @Override
            public void run() {
                for(int i=0; i<100; i++) {
                    try {
                        Thread.sleep(100);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    if(i<=50) {
                        singleton.addElement(Arrays.asList("ONE"));
                    } else {
                        singleton.addElement(Arrays.asList("TWO"));
                    }
                }
                
            }
        });

        Thread t2 = new Thread(new Runnable() {
            @Override
            public void run() {
                for(int i=0; i<100; i++) {
                    System.out.println(singleton.getStrList());
                    try {
                        Thread.sleep(100);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        });

        t1.start();
        t2.start();
        
    }
}

Output:

[TWO]
[]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]
[]
[TWO]
[TWO]
[TWO]
[TWO]
[TWO]

In between there are empty list returned. How to fix this without adding sleep when list is empty in the getStrList method?. Also looking for better alternate way to achieve this.

Upvotes: 0

Views: 262

Answers (3)

James Mudd
James Mudd

Reputation: 2373

You could replace the addElement method with

public void addElement(List<String> list) {
    List<String> newStrList = Collections.synchronizedList(new ArrayList<>(list));
    strList = newStrList;
}

This way the list would be replaced and you would never see the list in the empty state.

Upvotes: 0

RainerZufall
RainerZufall

Reputation: 153

I suppose the problem is your 'void addElement(List)' method.
It isn't synchronized.
Thread 1 get's priority and executes up to the line, where the list is cleared. Then Thread 2 get's priority and print's the status of the empty list. At last priority returns to Thread 1 and the item is added.

Synchronizing addElement should fix it.

Upvotes: 0

Vikas Verma
Vikas Verma

Reputation: 113

addElement has two statements:

strList.clear(); //statement1
strList.addAll(list); //statement2

getStrList() has one statement

Both the methods are non synchronized and due to which just before thread t2 is calling System.out.println(singleton.getStrList()) thread t1 has just executed statement1 which is clearing the list causing the t2 to print empty list.

To avoid it you have to make the addElement() and getStrList() as synchronized to fix the issue.

public synchronized void addElement(List<String> list) {
    strList.clear();
    strList.addAll(list);
    
}

public synchronized List<String> getStrList(){
    return new ArrayList<>(strList);
}

Upvotes: 1

Related Questions