ThreadWarden: detecting the scene graph being modified on the wrong thread

User Code & Projects
1

Having recently been hit with my least favourite exception java.lang.IllegalStateException: Scene graph is not properly updated for rendering. State was changed after rootNode.updateGeometricState() call. Make sure you do not modify the scene from another thread! Problem spatial name: null I’ve created a utility to give a bit more detail on what’s causing the problem (and where) and I though it may be useful to other people so I’ve released it as Thread Warden

The problem

When you accidentally modify the scene graph from the wrong thread JME may or may not notice (depending on how your threads interleave). If it does notice it will only tell you that you made a mistake (somewhere) and what the name of the spatial is (if you named your spatials; which I now do obsessively!). It doesn’t tell you where you did this, just that you did. These bugs kill my motivation and I usually take a day out after they hit because I know it will be such a slog to find the cause.

It is non trivial to avoid because this is a perfectly legal thing you might want to do

Future<Node> legalThreadingFuture = executorService.submit(() -> {
    //this is fine
    Node child2 = new Node("child2");
    Node child3 = new Node("child3");
    child2.attachChild(child3);
    return child2;
});

rootNode.attachChild(legalThreadingFuture.get());

Whereas this is illegal and may (or may not) cause JME to throw an assertion at some later point

Future<Void> illegalThreading = executorService.submit(() -> {
    Node child4= new Node("child4");
    rootNode.attachChild(child4);
    return null;
});

My Solution

I’ve used ByteBuddy to do bytecode rewriting and add instrumentation to the Node and Geometry classes. When a node (or geometry) gets a parent added it detects if that is part of the main scene. If it is it marks this spatial as being only allowed to be interacted with off the main thread (it does the same to all children). When the node is then mutated in any way later ThreadWarden checks the thread and raises an exception if the node should not be interacted with on the current thread.

This isn’t as bad for performance as I was expecting (I didn’t get a noticable FPS drop) but I’m sure there is some penalty. It goes without saying this is a debugging utility not intended to be included in releases

Examples of bugs this can find the source of

  • “Illegal Refresh FlagState: 1 for spatial null”
  • “Illegal refreshFlags RF_BOUND state for spatial null”
  • “Illegal light list update. Problem spatial name: null”
  • “Illegal mat param update. Problem spatial name: null”
  • “Illegal rf transform update. Problem spatial name: null”
  • java.lang.IllegalStateException: Scene graph is not properly updated for rendering. State was changed after rootNode.updateGeometricState() call. Make sure you do not modify the scene from another thread! Problem spatial name: null
  • State was changed after rootNode.updateGeometricState() call. Make sure you do not modify the scene from another thread!

How to use it

Include ThreadWarden as a dependency, in gradle this would be like this

implementation group: 'com.onemillionworlds', name: 'thread-warden', version: '1.0.1'

Then inside your applications register the root node as protected

public class MyApplication extends SimpleApplication{

    .....

    @Override
    public void simpleInitApp(){
        ThreadWarden.setup(rootNode);
        .....
    }
}

Then if an illegal scene graph mutation occurs then you will get a ThreadWardenException at the point the illegal mutation happens with a full stack trace to where in your code you made the mistake. And it will detect it the very first time it happens, not just if you get unlucky with how your threads interleave (which might not happen for a while during your testing)

(Depending on your java version you may need to run with -XX:+EnableDynamicAgentLoading)

11 Likes
2

This will be helpful for anyone not rigidly following near-clean-room level of thread management.

On the other side, if you want to manage your background thread to scene interaction better, I can highly recommend (personal bias) SiO2’s job state:

A Job makes the distinction between render thread and background thread a little clearer… though it’s still possible to abuse it:

public interface Job {
  void runOnWorker();    
  double runOnUpdate();
}

The big benefit here is that it has built-in avoidance of swamping the render thread. The naive approach to threading may end up adding 100s of things to the scene graph all at once. The JobState makes sure to split them up over subsequent frames based on a work factor.

4 Likes
3

That is very interesting about the adding geometries gradually. That is something I had thought about so I might investigate that.

However, I’ve found that the main thing that swamps the render thread (for me at least) is the first time calculation of collision shapes. But it is possible to explicitly request that happen (and so it can be done on the job thread not the render thread)

private void generateCollisionData(Spatial spatialToPrepare){
    if (spatialToPrepare instanceof Geometry geometry){
        geometry.getMesh().createCollisionData();
    }else if (spatialToPrepare instanceof Node) {
        for(Spatial child : ((Node)spatialToPrepare).getChildren()){
            generateCollisionData(child);
        }
    }
}

And that way by the time the geometry hits the scene the collision data already exists.

I assume the other thing that is a first-time-attach-swamp is pushing data to graphics memory, which I don’t think there is a way to do off thread so your job state may be a good solution for that!

The thing I got my IllegalStateException on (and created ThreadWarden to find) was exactly that sort of runOnWorker runOnUpdate thing, but I had (accidentally) reused something from an earlier creation of a sub scene and so it got detached from the root scene to create the new parts of the scene (and the hidden detachment from an earlier parent was what set JME off). Subtle bug that would have been a pain to find

2 Likes
4

Yes… and other scene book-keeping.

Mythruna adds a LOT of stuff all the time… and I generally override collideWith() so that collision data isn’t even considered for most of my geometry. The thing that was killing my frame rate (since the beginning I learned this lesson 13 years ago) was scene graph adds. Keep those below a certain threshold and your smooth as butter.

4 Likes
5

Wow!
Thank you @richtea and @pspeed, I really appreciate it! What you did will be of great help to me!

3 Likes