Skip to content

#2562 thread warden - #2563

Merged
yaRnMcDonuts merged 12 commits into
jMonkeyEngine:masterfrom
richardTingle:#2562-thread-warden
Nov 25, 2025
Merged

#2562 thread warden#2563
yaRnMcDonuts merged 12 commits into
jMonkeyEngine:masterfrom
richardTingle:#2562-thread-warden

Conversation

@richardTingle

@richardTingle richardTingle commented Sep 27, 2025

Copy link
Copy Markdown
Member

Purpose

The scene graph (that is connected to the root node) shouldn't be mutated from any thread but the main thread.

Unconnected nodes etc can be mutated on non main threads

What have I changed

Spatial mutations are now watched (when asserts are turned on) and if an illegal mutation is made an exception is fired

closes #2562

… in scene graph operations.

This update adds the SceneGraphThreadWarden to enforce that scene graph mutations occur only on the main thread IF those nodes are already part of the main scene graph
Introduce tests to verify thread safety of scene graph mutations under various conditions. Includes checks for both main-thread and non-main-thread scenarios with attached and detached objects. Ensures compliance with scene graph thread safety guarantees.
… handling

Added tests to validate thread safety in scenarios involving nested nodes, such as attaching/detaching children and grandchildren. Ensures proper restriction and release of thread protections for hierarchical structures. This strengthens coverage for complex scene graph operations.
…ort for disabling the thread warden via a system property.
…ice is being used where futures aren't promptly collected)

@riccardobl riccardobl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is going to be very useful.
I've commented some minor details I'd like you to review 👍

Comment thread jme3-core/src/main/java/com/jme3/app/SimpleApplication.java Outdated
Comment thread jme3-core/src/main/java/com/jme3/app/SimpleApplication.java Outdated
Comment thread jme3-core/src/main/java/com/jme3/scene/Geometry.java Outdated
Comment thread jme3-core/src/main/java/com/jme3/scene/Geometry.java Outdated
Comment thread jme3-core/src/main/java/com/jme3/scene/Geometry.java Outdated
@riccardobl

riccardobl commented Sep 27, 2025

Copy link
Copy Markdown
Member

Thanks! I am going to merge this asap on my fork, to give it some testing, just in case we are overlooking on some side effect.
But it looks good.

@yaRnMcDonuts yaRnMcDonuts added this to the v3.9.0 milestone Sep 30, 2025
@yaRnMcDonuts
yaRnMcDonuts merged commit e20665e into jMonkeyEngine:master Nov 25, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add assert checks for illegal scene graph mutations

3 participants