8 Replies Latest reply: Nov 26, 2010 2:58 AM by EJP RSS

    NullpointerException when using a looping thread

    815281
      I made a short tester program to illustrate this:
      abstract class ThreadTest implements Runnable
      {
          Thread thread;
          
          ThreadTest()
          {
              thread = new Thread(this);
              thread.start();
          }
      
          public void run()
          {
              while (true)
              {
                  update();
                  try { Thread.sleep(500); } catch (InterruptedException e) {}
              }
          }
          
          abstract void update();
          
          public static void main(String[] args)
          {
              new SubTest();
          }
      }
      class SubTest extends ThreadTest
      {
          Person p = new Person();
          void update()
          {
              //fixes the NullPointerException.  
              //if (p != null)
                  p.update();
          }
      }
      class Person
      {
          int age = 0;
          
          void update()
          {
              age++;
              System.out.println(age);
              if (age >= 5)
                  System.exit(0);
          }
      }
      SubTest's update method will regernate a Nullpointerexception when running at this state because reference p is apperently null when runned. But how can reference p ever be null really? When a new object of SubTest is created reference p should directly be initialized a value (new Person) because its written in class definition?

      I noted that this can be worked around by adding
      if (p != null)
      to check if p is null, and if so, skip updating this round. What am I missing to understand here? Is the update method called in SubTest before like the whole "creation" of the object is complete so for a short time p equals null instead a of a Person object?

      A bigger project of mine is built up on this kind of thread-looping and I have to to put in if-statements everywhere to check if references really pointing at a object when they are used. This feels like a bad way doing it, how can I solve this in a more "corect way"?

      Thanks in advance!

      Same question discussed:
      http://forum.codecall.net/java-help/34347-nullpointerexception-when-using-looping-thread.html#post282474
      http://www.coderanch.com/t/518436/java/java/Nullpointerexception-when-looping-thread#2347166

      Edited by: 812278 on Nov 25, 2010 1:58 PM
        • 1. Re: NullpointerException when using a looping thread
          DrClap
          Cross-posted on CodeRanch.
          • 2. Re: NullpointerException when using a looping thread
            815281
            links added.
            • 3. Re: NullpointerException when using a looping thread
              EJP
              The thread is starting to execute before the constructor for SubTest has finished doing its work, so it is possible for SubTest.run() to be entered before 'p' has been assigned. Basically what is happening is this:
              SubTest() // constructor starts
              super(); // calls ThreadTest()
              Thread.start(); // called by ThreadTest();
              run(); // called by the system when the thread starts
              update();
              p == null;
              // meanwhile control returns to SubTest
              p = new Person();
              // remainder of SubTest(), in this case nothing, bearing in mind that it's auto generated by the compiler.
              Moral: don't call Thread.start() inside constructors.
              • 4. Re: NullpointerException when using a looping thread
                815281
                EJP wrote:
                The thread is starting to execute before the constructor for SubTest has finished doing its work, so it is possible for SubTest.run() to be entered before 'p' has been assigned. Basically what is happening is this:
                SubTest() // constructor starts
                super(); // calls ThreadTest()
                Thread.start(); // called by ThreadTest();
                run(); // called by the system when the thread starts
                update();
                p == null;
                // meanwhile control returns to SubTest
                p = new Person();
                // remainder of SubTest(), in this case nothing, bearing in mind that it's auto generated by the compiler.
                Moral: don't call Thread.start() inside constructors.
                But if you put the thread.start() in the end of SubTest's constructor it should be fine, I think.

                ---------------------------------------------------------------------------------------------------------------------------------------
                I've read the response at JavaRanch. They are correct.
                Either you put the if-statement back there.
                or you don't do .start() in the constructor of ThreadTest. (Either in the main, or in the SubTest constructor)

                Since the class is abstract and will always be extended, to me it's more likely to put the .start in the subclass.

                --------------------------------------------------------------------------------------------------------------------------------------
                Thank you, adding thread.start() at the end of SubTest's constructor seems to solve it nice and good without having to use a if-statements to check p.
                --------------------------------------------------------------------------------------------------------------------------------------
                oxano on Codecall mentioned that due to Henry's good summary of how a object is instantiated its better to do the
                thread.start();
                in constructor of SubTest instead of the superclass ThreadTest.
                Then if you make sure you put that start command of the thread in the very end of SubTest's constructor, you can be sure that step 1, 2 and 3 are done before the thread starting going, that in this case use a reference that might cause a NullpointerException if not initialized yet.

                After testing that it worked fine. Just to make clear what changes I made:
                abstract class ThreadTest implements Runnable
                {
                    Thread thread;
                    
                    ThreadTest()
                    {
                        thread = new Thread(this);
                    }
                
                    public void run()
                    {
                        while (true)
                        {
                            update();
                            try { Thread.sleep(500); } catch (InterruptedException e) {}
                        }
                    }
                    
                    abstract void update();
                    
                    public static void main(String[] args)
                    {
                        new SubTest();
                    }
                }
                class SubTest extends ThreadTest
                {
                    Person p = new Person();
                    
                    SubTest()
                    {
                        thread.start();
                    }
                    void update()
                    {
                        p.update();
                    }
                }
                • 5. Re: NullpointerException when using a looping thread
                  815281
                  Unfortunately the rule is: Don't call a method which can be overridden from a constructor. If you do, then you risk the problem you had here, and it can happen even with no threading involved. Generally that means removing those method calls from the constructor and putting them in some new method which you would call after creating the object.
                  ----------------------------------------------------------------------------------------------------------------------------
                  You mean that you don't risk suffer from NullpointerExceptions if you add
                  thread.start();
                  (which is putted in a method we can call "start") after
                  new SubTest();
                  in main? I understand that its not because of the threading in this case you do so, but generally calling methods from above in a constructor. Well, if I understand what you said correctly.
                  Like this then maybe? As I didn't need any constructor I removed them to save space, the compiler add empty ones invisible.
                  abstract class ThreadTest implements Runnable
                  {
                       Thread thread = new Thread(this);
                  
                       public void run()
                       {
                            while (true)
                            {
                                 update();
                                 try { Thread.sleep(500); } catch (InterruptedException e) {}
                            }
                       }
                       
                       abstract void update();
                       abstract void start();
                       
                       public static void main(String[] args)
                       {
                            SubTest sub = new SubTest();
                            sub.start();
                       }
                  }
                  class SubTest extends ThreadTest
                  {
                       Person p = new Person();
                       
                       void update()
                       {
                            p.update();
                       }
                       
                       void start()
                       {
                            thread.start();
                       }
                  }
                  -------------------------------------------------------------------------------------------------------------------

                  As you said EJP, removing the thread.start() call from constructor is a better way doing it, even if I you don't seem to get any problems in this case.
                  • 6. Re: NullpointerException when using a looping thread
                    EJP
                    I don't get the point of making it abstract in the base class and implementing it in the derived class when it is the base class that has the Thread member. That's just pointless complication. The main thing is to get Thread.start() away from the constructors and you've done that.
                    • 7. Re: NullpointerException when using a looping thread
                      815281
                      EJP wrote:
                      I don't get the point of making it abstract in the base class and implementing it in the derived class when it is the base class that has the Thread member. That's just pointless complication. The main thing is to get Thread.start() away from the constructors and you've done that.
                      Oh ok, so the abstract start method isn't really necessary?
                      abstract class ThreadTest implements Runnable
                      {
                           Thread thread = new Thread(this);
                      
                           public void run()
                           {
                                while (true)
                                {
                                     update();
                                     try { Thread.sleep(500); } catch (InterruptedException e) {}
                                }
                           }
                           
                           abstract void update();
                           
                           public static void main(String[] args)
                           {
                                new SubTest().thread.start();
                           }
                      }
                      class SubTest extends ThreadTest
                      {
                           Person p = new Person();
                           
                           void update()
                           {
                                p.update();
                           }     
                      }
                      • 8. Re: NullpointerException when using a looping thread
                        EJP
                        Rather than exposing 'thread' I would have just moved the implementation of start() into the base class.