Cannot delegate JavaFx Task to a new Thread in a static method

Question

I've a General class which has static methods and static variables with Hibernate configuration settings and methods which return List after hitting the database. I am working on JavaFx and i recently learned its better to use Task for time consuming operations like hitting the db for a long list of data etc. So, here in my case, i created a Task object, wrote the code in anonymous inner class which contains the code for hitting the db for the LOGIN credential. The Task must return an instance of List.

I intialized a Thread object inside the static method, passed Task's object in its constructor, set the daemon to true and started the Thread. However i am getting NullPointerException after running the application..

private static SessionFactory sessionFactory = null;
private static Session session = null;
private static final Transaction transaction = null;
private static final Configuration configuration = null;
private static List list;
  public static List getSelectList(final String query)
  {
      //list = null;
      System.err.println("Inside getSelectList");
      try{
      final Task <List> task= new Task <List> ()
      {
          @Override
          public List call()
          {
              try
              {
                 System.err.println("Inside call");
                 session.beginTransaction();
                 list = session.createQuery(query).list();
                 System.err.println(list);


                 session.getTransaction().commit();
                 return list;
              }
                catch (Exception e)
                {
                     session.getTransaction().rollback();
                     e.printStackTrace();
                }
              System.err.println("Outta try block");
              return null;
          }
      };

      task.setOnSucceeded(new EventHandler<WorkerStateEvent>() {
          @Override
          public void handle(WorkerStateEvent t) {
              System.err.println("Inside handle");
             list = task.getValue();
            /* for (Iterator it = list.iterator();it.hasNext(); )
                 {
                  System.err.println("Inside Set on succeeded");
                  //System.err.println( ((Login)it.next()).getUsername());   
                 } */
             }
            });

     Thread th = new Thread(task);
     th.setDaemon(true);
     th.start();


     //list=task.getValue();

      }
      catch (Exception e) {e.printStackTrace();}
      return list;

  }

Exception which i got

However, after many combinations and random permutations, things worked for me with commenting the Thread code and instead substituting it with task.run();

 /*Thread th = new Thread(task);
     th.setDaemon(true);
     th.start();*/
task.run();


 try
              {

                 session.beginTransaction();
                 //List locallist = session.createSQLQuery(query).list();
                 list = session.createSQLQuery(query).list();
                 session.getTransaction().commit();
                 return list ;
              }
                catch (Exception e){
                     session.getTransaction().rollback();

I wish to know why this happened. Why task.run() along with using static List worked for me?

Converting List to static and removing the Thread instance and using task.run(); was merely a number to different tries from my side to make my code work. i dunno the reasons. I will be greatly humbled for any explanations. Thanks in advance!

James_D · Accepted Answer · 2015-04-08T18:39:08.963

What is happening

Creating a Thread with your task, and then calling start() on the thread causes task.run() to be executed asynchronously in the thread you created.

Just calling task.run() executes the run() method in the current thread.

The code you have with the thread is non-deterministic in its behavior. In other words, you cannot predict from the code alone what the result will be. The issue is that you are accessing the shared list instance from two different threads, with no control over the order of access. Here is what happens:

list is initially null.

You call getSelectList(). In getSelectList():

You create a task.
You configure the task to set the value of list to the result of the query when the task completes. (This will happen on the FX Application Thread.)
You create a new thread which will execute the task.
You launch the thread, causing the task to be executed asynchronously
You return the value of list

Because the task is executing asynchronously, you have no control over whether or not the task completes before the getSelectList() reaches its return statement.

So if getSelectList() reaches its return statement before the task completes (and before the onSucceeded handler for the task is invoked), getSelectList() will return the value of list before the task updates it, i.e. it will return null. This is almost certainly more likely to happen (because the task is accessing a database, which is slow), and I expect this is why you get the null pointer exception.

If the task happens to complete and complete the invocation of its onSucceeded handler before the getSelectList() reaches its return statement, then by the time getSelectList() reaches the return statement, list will have been updated and it returns the value set by the task. This is highly unlikely, and even if it happens, there's still no actual guarantee you get the "live" value of list (due to some complexities in the Java Language Specification about the relationship between threads and memory).

Note that if you invoke getSelectList() from the FX Application Thread, then you are guaranteed that it will return null, because the onSucceeded handler cannot possibly be invoked before getSelectList() completes (because those two method invocations are both running on the same thread - the FX Application Thread).

How to fix it

First, you should avoid accessing a shared variable (list) from different threads, unless you have proper synchronization. Synchronizing yourself is difficult and you should typically use a high-level API to manage that for you. The Task API is designed for this (along with the general java.util.concurrent API).

I generally avoid managing threading (or exception handling) in a Data Access Object class, and just let the client code wrap the calls to the DAO in threading code if they need.

So (I am not going to make this method static as that is generally horrible):

public List getSelectList(String query) throws Exception {

    Session session = sessionFactory.createSession();

    try {
        session.beginTransaction();
        List list = session.createQuery(query).list();
        session.getTransaction().commit();
        return list ;
    } catch (Exception e) {
        Transaction tx = session.getTransaction();
        if (tx != null) {
            tx.rollback();
        }
        throw e ;
    }
}

Then, from your JavaFX UI code, you can do

DAO myDAO = ... ;

Task<List> task = new Task<List>() {
    @Override
    public void call() throws Exception {
        return myDAO.getSelectList(...);
    }
});

task.setOnSucceeded(event -> {
    List list = task.getValue();
    // use list to update UI...
});

task.setOnFailed(event -> {
    Exception exc = task.getException();
    // handle exception...
});

Thread thread = new Thread(task);
thread.setDaemon(true);
thread.start();

If you really want the DAO method to run asynchronously, you need to supply callbacks to it to be executed when it succeeds or fails. So:

public void getSelectList(String query, 
    Consumer<List> succeededHandler, 
    Consumer<Exception> errorHandler) {

    FutureTask<List> futureTask = new FutureTask<>(() -> {

        Session session = sessionFactory.getSession();
        try {
            session.beginTransaction();
            List list = session.createQuery(query).list();
            session.getTransaction().commit();
            return list ;
       } catch (Exception e) {
            Transaction tx = session.getTransaction();
            if (tx != null) {
                tx.rollback();
            }
            throw e ;
       }
    });

    Thread thread = new Thread(futureTask);
    thread.setDaemon(true);
    thread.start();

    try {
        List list = futureTask.get();
        succeededHandler.accept(list);
    } catch (Exception e) {
        errorHandler.accept(e);
    }
}

Now from your UI you do something like:

DAO myDAO = ... ;
String query =  ... ;

myDAO.getSelectList(query, 
    list -> Platform.runLater(() -> {
        // update UI with list ...
    }),
    exc -> Platform.runLater(() -> {
        // handle exception...
    })
);

Further improvements

You should use a properly generically-typed List, rather than a raw type, for type safety.
Use an Executor instead of managing the thread creation yourself.

Caveat All code was just typed in here as-is, without testing, so there may be typos. It should give you the idea though.

I thank you a lot for the beautiful explanation u provided! Thanks again budd! — Lalit Rao, Apr 08 '15 at 19:58

Cannot delegate JavaFx Task to a new Thread in a static method

1 Answers1