search

Renanse / Ardor3D

Ardor3D is a free Java based, professionally oriented, open source 3D graphics engine.
Other
227 stars 77 forks source link

Added check for empty child list #112

Closed jgyorfi closed 6 months ago

jgyorfi commented 6 months ago

Endurance testing of VERVE (i.e., leaving it running overnight) uncovered a strange problem in which index-out-of-bounds errors were being thrown in the Node.getChild(int) method that caused Ardor and VERVE to crash. Somehow child lists that shouldn't be empty had become empty, so no matter what the integer argument was, an error would be thrown. The proposed change simply checks if the list is empty and if so, the return value will be null. There haven't been any crashes since making this change.

Renanse commented 6 months ago

I am a little uncomfortable with this change as it hides the root cause rather than addressing it - if a Node is truly losing children, this is bizarre and troubling behavior. I would suggest that the number of children a Node has is probably being cached somehow (perhaps in a loop that is able to remove/detach children, for example).

jgyorfi commented 6 months ago

I agree that this is a bandage treating a sympton without addressing the root cause, but the failures all occurred overnight when nothing should have been happening. With no one there, there should have been no input and therefore no change to the scenegraph. We have no idea what could be causing the problem, we just tried this as a work-around and it seemed to fix the problem. If you have any suggestions as to how we could try to debug the root cause, please let us know.

Renanse commented 6 months ago

Fair enough, I know how tricky these things can be... perhaps some sort of telemetry around Node children operations? I know VERVE was idling, but I assume there's still some operations occurring - heartbeats, changes to planetary positions, etc? I'll fold this PR in, but it feels like you may end up finding something else later that you'll trace back to this same area :)

Renanse commented 6 months ago

Nice to see you on here, btw :)

jgyorfi commented 6 months ago

Thanks for merging -- I just want to keep the mainline up-to-date with what we're doing. It's nice to be working together again, even if only for a one-line change.

Renanse commented 6 months ago

I've been doing some code cleanup on a branch and would love to have eyes on it at some point if the team has time. All the best!