CODEWITHSUNDEEP
javamultithreadinginitialization
Gmfreaky
Gmfreaky

Reputation: 67

Thread execution order Java

PhysicsThread.java starts a new thread. The run method executes the initialize method. This method creates the physics world. After that, it starts a while loop that updates the world with an interval of 1000/60 = 16 milliseconds. However, I get a nullpointerexception in the loop, because it does not always know world, even though I check for that. Can explain what is happening?

PhysicsThread.java

public class PhysicsThread implements Runnable {

long fps = 60;
DiscreteDynamicsWorld dw;
long lasttime = System.currentTimeMillis();;

static int maxPhysicsObjects = 50000;

PhysicsThread() {
    Thread t = new Thread(this);
    t.setPriority(Thread.MIN_PRIORITY);
    t.start();
}

@Override
public void run() {
    System.out.println("Start thread");
    init();
    System.out.println("Start threadloop");

    while(true) {
        loop();
    }
}

public void init() {
    BroadphaseInterface broadphase = new AxisSweep3_32(new Vector3f(0,0,0), new Vector3f(200000,200000,200000), maxPhysicsObjects);//new DbvtBroadphase();
    DefaultCollisionConfiguration collisionConfiguration = new DefaultCollisionConfiguration();
    CollisionDispatcher dispatcher = new CollisionDispatcher(
            collisionConfiguration);

    SequentialImpulseConstraintSolver solver = new SequentialImpulseConstraintSolver();

    dw = new DiscreteDynamicsWorld(dispatcher, broadphase,
            solver, collisionConfiguration);
    System.out.println("Made world");
    dw.setGravity(new Vector3f(0, 0, -10));
}

public void loop() {
    if (dw!=null) {
        float delta = System.currentTimeMillis()-lasttime;
        lasttime = System.currentTimeMillis();
        dw.stepSimulation(delta/1000f, 1, 1/60f);
    }
    try {
        Thread.sleep(1000/fps);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

public DiscreteDynamicsWorld getWorld() {
    return dw;
}

}

Upvotes: 2

Views: 635

Answers (1)

Andrzej Doyle
Andrzej Doyle

Reputation: 103797

I suspect this may be down to your construction technique, which is an antipattern. You shouldn't let the this reference "escape" during a constructor, because then other threads may see the object in a partially-constructed state. At this point, all bets are off about the state of the class, and even invariants that are always true (such as final fields being set to a value) may not hold.

I wonder if what is happening here might be due to the following sequence of events:

  • The constructor is called and starts the new thread based on this.
  • The new thread runs, and fully executes init (setting dw), and half of loop, before being pre-empted.
  • The main thread continues with the constructor, and sets dw to null as part of field initialisation.
  • The spawned thread continues after working out delta and lastTime, then sees a null value for dw.

I'm a little hesistant to say that's exactly the case because I would have expected the fields to be initialised before the constructor body runs. However it seems likely that accessing the fields while the object is still being constructed is a very bad idea.

In order to fix this I'd suggest not starting a thread in the constructor, but rather having the calling code start the thread afterwards, e.g.:

final PhysicsThread pt = new PhysicsThread();
final Thread t = new Thread(pt);
t.setPriority(Thread.MIN_PRIORITY);
t.start();

(Also, the idea of having init methods that aren't the constructor is usually a bad idea - if the dw field never changes, why not perform all the work of init during the constructor itself and mark dw as final? This will be cleaner and make your code easier to reason about, since the VM guarantees the value will be fully set during construction and that it will never change afterwards. The only downside is a performance hit if you construct lots of these instances and never actually run them - so just don't do that. :)

Plus the class name is misleading, since this isn't a Thread but a Runnable. Calling it something like PhysicsTask or PhysicsSimulationRun would arguably be clearer.)

Upvotes: 3

Related Questions