jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.78k stars 1.12k forks source link

NPE in GLRenderer.clearVertexAttribs() #1969

Closed stephengold closed 1 year ago

stephengold commented 1 year ago

At the Discourse Forum/Hub today, Nikki reported a crash in JME v3.5.2:

SEVERE: Uncaught exception thrown in Thread[jME3 Main,5,main]
java.lang.NullPointerException
    at com.jme3.renderer.opengl.GLRenderer.clearVertexAttribs(GLRenderer.java:2911)
    at com.jme3.renderer.opengl.GLRenderer.renderMeshDefault(GLRenderer.java:3245)
    at com.jme3.renderer.opengl.GLRenderer.renderMesh(GLRenderer.java:3276)
    at com.jme3.material.logic.DefaultTechniqueDefLogic.renderMeshFromGeometry(DefaultTechniqueDefLogic.java:72)
    at com.jme3.material.logic.DefaultTechniqueDefLogic.render(DefaultTechniqueDefLogic.java:97)
    at com.jme3.material.Technique.render(Technique.java:167)
    at com.jme3.material.Material.render(Material.java:1052)
    at com.jme3.renderer.RenderManager.renderGeometry(RenderManager.java:651)
    at com.jme3.renderer.queue.RenderQueue.renderGeometryList(RenderQueue.java:273)
    at com.jme3.renderer.queue.RenderQueue.renderQueue(RenderQueue.java:315)
    at com.jme3.renderer.RenderManager.renderViewPortQueues(RenderManager.java:936)
    at com.jme3.renderer.RenderManager.flushQueue(RenderManager.java:823)
    at com.jme3.renderer.RenderManager.renderViewPort(RenderManager.java:1184)
    at com.jme3.renderer.RenderManager.render(RenderManager.java:1248)
    at com.jme3.app.SimpleApplication.update(SimpleApplication.java:278)
    at com.jme3.system.lwjgl.LwjglAbstractDisplay.runLoop(LwjglAbstractDisplay.java:160)
    at com.jme3.system.lwjgl.LwjglDisplay.runLoop(LwjglDisplay.java:201)
    at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:242)
    at java.base/java.lang.Thread.run(Thread.java:829)

Using the test case provided by Nikki, I found that a very similar crash occurs with the latest "master" branch of JME. I don't know what the test app actually does to trigger the crash.

As expected, boundAttribs[idx] is null at the point of failure. However, the null is not caused by duplicate indices. Here are some values at the start of the final invocation of clearVertexAttribs():

oldLen = 1
oldList[0] = 1
boundAttribs[1] = null

According to the javadoc for boundAttribs, null array elements are legal: https://github.com/jMonkeyEngine/jmonkeyengine/blob/8867934fddcf577dd75b8db02f2b3f071e6a8513/jme3-core/src/main/java/com/jme3/renderer/RenderContext.java#L310-L314

After I add a null check (as below), the test app runs without crashing.

            WeakReference<VertexBuffer> ref = context.boundAttribs[idx];
            if (ref != null) {
                VertexBuffer buffer = ref.get();
                if (buffer != null && buffer.isInstanced()) {
                    glext.glVertexAttribDivisorARB(idx, 0);
                }
            }

I suppose this check was overlooked at the time of PR 1408. Given that 1408 was integrated into JME v3.4---and how often clearVertexAttribs() gets invoked---it's very surprising nobody reported the crash before.

Ali-RS commented 1 year ago

Thanks for recording this issue and submitting a fix.