Jump to content




Recent Status Updates

  • Photo
      30 Sep
    rhossis

    laptop hard disk seated beneath motherboard but with no access panel. 7 hours to replace :(

    Show comments (3)
  • Photo
      19 Sep
    Chall

    I love it when you go to write a help thread, then while writing, you reach an enlightenment, and figure it out yourself.

    Show comments (3)
View All Updates

Developed by Kemal Taskin
Photo
- - - - -

Nullpointerexception when using a looping thread

loop null

  • Please log in to reply
5 replies to this topic

#1 Cander

Cander

    CC Resident

  • Advanced Member
  • PipPipPipPip
  • 55 posts

Posted 25 November 2010 - 01:33 PM

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:
Nullpointerexception when using a looping thread (Java in General forum at JavaRanch)
OTN Discussion Forums : NullpointerException when using a ...

Edited by Cander, 25 November 2010 - 02:10 PM.

  • 0

#2 wim DC

wim DC

    Roar

  • Expert Member
  • PipPipPipPipPipPipPipPip
  • 2,574 posts
  • Programming Language:Java, JavaScript, PL/SQL
  • Learning:Java, PHP

Posted 25 November 2010 - 02:32 PM

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.
  • 0

#3 Cander

Cander

    CC Resident

  • Advanced Member
  • PipPipPipPip
  • 55 posts

Posted 25 November 2010 - 03:26 PM

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();
    }
}


  • 0

#4 Cander

Cander

    CC Resident

  • Advanced Member
  • PipPipPipPip
  • 55 posts

Posted 25 November 2010 - 04:13 PM

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 he says that maybe its a bad way doing it by adding thread.start() in the constructor of SubTest because it can cause other problems in other cases. Above I tried an other version of the program calling thread.start() completely after the SubTest object was instantiated in main. As you already mentioned oxano by the way yeah.
  • 0

#5 wim DC

wim DC

    Roar

  • Expert Member
  • PipPipPipPipPipPipPipPip
  • 2,574 posts
  • Programming Language:Java, JavaScript, PL/SQL
  • Learning:Java, PHP

Posted 25 November 2010 - 11:10 PM

If you don't need the thread object later, to stop it for example, you can write it on 1 line :)
New SubTest().start();

  • 0

#6 Cander

Cander

    CC Resident

  • Advanced Member
  • PipPipPipPip
  • 55 posts

Posted 26 November 2010 - 04:24 AM

If you don't need the thread object later, to stop it for example, you can write it on 1 line :)

New SubTest().start();


Yes of course, that's smart

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();
    }    
}

Optimized the code a bit I think.
  • 0





Also tagged with one or more of these keywords: loop, null