Caerbannog
Caerbannog

Reputation: 41

Java - LinkedList : Weird NullPointerException

I have a little problem with a NullPointerException that I can't really understand.

My code runs 24/7 and works pretty well but I have that exception that pops randomly from 1 day to 1 week after the application is started.

Here is the stacktrace :

java.lang.NullPointerException
at java.util.LinkedList.get(LinkedList.java:477)
at com.ch4process.acquisition.ScenarioWorker.eventHandling(ScenarioWorker.java:97)
at com.ch4process.acquisition.ScenarioWorker.call(ScenarioWorker.java:79)
at com.ch4process.acquisition.ScenarioWorker.call(ScenarioWorker.java:1)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)

As you can see this exception is rised in a thread.

Here is the code (simplified a bit) :

public class ScenarioWorker implement Callable<Integer>
{
List<SignalValueEvent> eventList = new LinkedList<>();

boolean busy = false;

@Override
public Integer call() throws Exception
{
    try
    {
        while (true)
        {
            eventHandling();
            Thread.sleep(1000);
        }
    }
    catch (Exception ex)
    {
        // Redirects the exception to a custom class
    }
}

private void eventHandling()
{
    if (! busy)
    {
        while (eventList.size() > 0)
        {       
            SignalValueEvent event = eventList.get(0); // NullPointerException here according to stacktrace

            if(event.isTriggered()))
            {
                busy = true;
                doScenario(event);
            }
            deleteEvent();
        }
    }
}


private void deleteEvent()
{
    try
    {
        eventList.remove(0);
    }
    catch (Exception ex)
    {
        // Redirects the exception to custom class
    }
    finally
    {
        busy = false;
    }
}


@Override
public void SignalValueChanged(SignalValueEvent event)
{
    if (event.isValid())
    {
        eventList.add(event);
    }
}

}

[Edit : the line 97 in the stacktrace is the line saying SignalValueEvent event = eventList.get(0); ]

This class implements an interface which allows another class to notify it by calling the SignalValueChanged method.

So basically my LinkedList is created at the initialization of the class, filled by an external class whenever an event needs to be put in the list, and the call method loops on the list to see if there's anything in it. If there is, it's treated and the event is deleted.

I've tested this and the only reason I should have a NPException is if my list is equal to null... but I don't do that anywhere in my code...

As I said this code is working 24/7 and I had this bug almost one week and a half after I started the application. Is it something obvious I am missing ?

Thank you so much for reading this and if you can help me I'll be glad :)

Upvotes: 3

Views: 2385

Answers (4)

jacobm
jacobm

Reputation: 14025

The NPE isn't because your list is null -- if it were, the root of your exception would be on ScenarioWorker.java line 97. Instead the code is making it into the internals of LinkedList, which indicates that something internal to the LinkedList implementation is getting screwed up -- a huge red flag for concurrency problems. As for why, probably a race in one thread calling SignalValueChanged and another calling eventHandling at the same time.

You can solve it by synchronizing all access to eventList. The easiest way is probably just to mark the methods SignalValueChanged and eventHandling as synchronized.

Upvotes: 4

Markus Mitterauer
Markus Mitterauer

Reputation: 1610

Just read LinkedLists JavaDoc:

Note that this implementation is not synchronized. If multiple threads access a linked list concurrently, and at least one of the threads modifies the list structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more elements; merely setting the value of an element is not a structural modification.) This is typically accomplished by synchronizing on some object that naturally encapsulates the list. If no such object exists, the list should be "wrapped" using the Collections.synchronizedList method. This is best done at creation time, to prevent accidental unsynchronized access to the list:

List list = Collections.synchronizedList(new LinkedList(...));

Upvotes: 2

shiri
shiri

Reputation: 745

As Hovercraft Full Of Eels suggests above, I'm guessing that this is a concurrency issue.

Most likely, at the time that you're calling eventList.get(0), despite having "just" tested that the list is not empty, either the list became null or the list became empty, via another concurrent thread.

Edit: As discussed in the comments to the question, the NullPointerException is being thrown from the inner workings of LinkedList, meaning it's most definitely a concurrency issue.

Make your objects thread-safe, and you will solve the problem.

Upvotes: 1

AhmadWabbi
AhmadWabbi

Reputation: 2197

I think this is related to threads synchronization too. Solution: Encapsulate eventList in an object with synchronized methods. Access eventList from all threads by calling these synchronized methods only.

Upvotes: 3

Related Questions