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.


Solution 1:

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);
}