CODEWITHSUNDEEP
java
wtsang02
wtsang02

Reputation: 18863

NoSuchElementException even after check

for (final ArrayList<SmartPhone> smartPhones : smartPhonesCluster) {
    new Thread(new Runnable() {
        @Override
        public void run() {
            for (SmartPhone smartPhone : smartPhones) {
                Queue<SmartPhoneTask> tasks = smartPhone.getSystem()
                                                        .getTaskQue();
                SmartPhoneTask task = null;
                assert tasks != null;
                try {
                    while (!tasks.isEmpty()) {
                        task = tasks.poll(); // This is the line throwing the exception (GlobalNetwork.java:118)
                        assert task != null;
                        task.execute();
                        task.onTaskComplete();
                    }
                } catch (RuntimeException e) {
                    e.printStackTrace();
                }
            }
        }
    }).start();
}

And log:

java.util.NoSuchElementException
    at java.util.LinkedList.remove(LinkedList.java:788)
    at java.util.LinkedList.removeFirst(LinkedList.java:134)
    at java.util.LinkedList.poll(LinkedList.java:470)
    at com.wtsang02.reu.botnet.network.GlobalNetwork$1.run(GlobalNetwork.java:118)
    at java.lang.Thread.run(Thread.java:662)
java.lang.NullPointerException
Exception in thread "Thread-299" java.lang.AssertionError
    at com.wtsang02.reu.botnet.network.GlobalNetwork$1.run(GlobalNetwork.java:119)
    at java.lang.Thread.run(Thread.java:662)

line 118 points to:

task=tasks.poll();

How to solve this? Queue is LinkedList implemenation if that makes a difference.

Upvotes: 5

Views: 6734

Answers (1)

yshavit
yshavit

Reputation: 43391

LinkedList is not thread-safe, so you need external synchronization if you access a Linkedlist on more than one thread. This synchronization is on some object (a synchronized method is just shorthand for "synchronize on this"), and both the gets and the puts must be synchronized on the same object. You're definitely doing that here, since you create a new thread for each SmartPhone, and then access that phone's LinkedList from there.

If one thread puts into the list while synchronized on someObject1, and then another thread reads that list while synchronized on someObject2, then this does not count as external synchronization -- the code is still broken.

Even if you used a thread-safe collection, it'd be possible to hit this exception if multiple threads are emptying the queue at the same time. For instance, imagine this:

thread A: put e into queue1
thread B: queue1.isEmpty()? No, so go on
thread C: queue1.isEmpty()? No, so go on
thread B: queue1.poll() // works
thread C: queue1.poll() // NoSuchElementException

You should use BlockingQueue, whose poll() method will return null if there are no more elements in the list. Keep pulling until you get a null, and then break the loop.

Upvotes: 10

Related Questions