CODEWITHSUNDEEP
javamultithreadinglistiteratorthread-safety
zkristic
zkristic

Reputation: 639

Thread safe remove/add element from one list to another

Let's say I have two lists: fooList, and barList. Also, let's say I have two threads: first one iterates over fooList and if certain criteria is met (condition is true) it removes element from fooList and adds it to barList. Second one iterates over barList, and if some other condition is true it removes element from barList, and adds it to fooList.

The way I handled it is:

private static Object sharedLock = new Object();

Thread t1 = new Thread() {
    public void run() {
        synchronized (sharedLock) {

            for (Iterator<String> iterator = fooList.iterator(); iterator.hasNext();) {
                String fooElement = iterator.next();
                if (condition == true) {

                    iterator.remove();
                    barList.add(fooElement);

                }
            }

        }
    }
};

Thread t2 = new Thread() {
    public void run() {
        synchronized (sharedLock) {

            for (Iterator<String> iterator = barList.iterator(); iterator.hasNext();) {
                String barElement = iterator.next();
                if (otherCondition == true) {

                    iterator.remove();
                    fooList.add(barElement);

                }
            }

        }
    }
};

What I want to know is have I handled it properly? Is there a race condition possibility? Is there a better way to achieve the same functionality?

EDIT Looks like proper way of implementing this is:

Thread t1 = new Thread() {
    public void run() {

        for (String fooElement : fooList) {
            if (condition == true) {

                fooList.remove(fooElement);
                barList.add(fooElement);

            }
        }

    }
};

Thread t2 = new Thread() {
    public void run() {

        for (String barElement : barList) {
            if (otherCondition == true) {

                barList.remove(barElement);
                fooList.add(barElement);

            }
        }

    }
};

where both: fooList and barList are of type CopyOnWriteArrayList<String>

Upvotes: 1

Views: 3148

Answers (2)

jhkuperus
jhkuperus

Reputation: 1469

The way you implemented it now, t1 and t2 will run sequentially and not parallel. Whichever one starts first, claims the lock, performs his entire loop, terminates and releases the lock for the other.

The good thing is: there is no race condition. The bad thing is: there is no parallelism.

In general, it's a bad idea to work directly with locks if you can avoid it. Java contains a load of collections specifically for concurrent use. See Java Concurrency Utils

Upvotes: 2

Bohemian
Bohemian

Reputation: 425073

Don't reinvent the wheel: use a threadsafe implementation of List from the JDK:

List<String> fooList = new CopyOnWriteArrayList<>();

See javadoc

Upvotes: 9

Related Questions