IllegalArgumentException in registerObject() #1982

stephengold · 2023-03-07T21:05:42Z

This issue was reported at the Discourse hub/forum: https://hub.jmonkeyengine.org/t/java-lang-illegalargumentexception-object-id-must-be-greater-than-zero-on-mac/46559

jmonkeyengine/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java

Lines 104 to 107 in 44a50de

    
           public void registerObject(NativeObject obj) { 
        
               if (obj.getId() <= 0) { 
        
                   throw new IllegalArgumentException("object id must be greater than zero"); 
        
               }

While JME specifically reserves the value -1 for invalid IDs:

jmonkeyengine/jme3-core/src/main/java/com/jme3/util/NativeObject.java

Line 47 in 44a50de

public static final int INVALID_ID = -1;

I believe negative IDs in general don't justify throwing an exception.

The OpenGL documentation doesn't specify range of values for a texture name, only that they are GLuint:

According to the OpenGL spec, GLuint is at least 32 bits, so values > 0x7fffffff will produce negative values in a Java int.

I think the tests in lines 105 and 132 should be specifically for == INVALID_ID instead of <= 0.

The text was updated successfully, but these errors were encountered:

stephengold · 2023-03-07T21:18:49Z

The GLRenderer class appears to have similar issues. In some places, it checks against -1; in others, it requires > 0.

jmonkeyengine/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java

Line 1254 in 44a50de

assert shader.getId() > 0;
jmonkeyengine/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java

Line 1364 in 44a50de

assert shader.getId() > 0;
jmonkeyengine/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java

Lines 1455 to 1456 in 44a50de

int id = source.getId();

if (id == -1) {
jmonkeyengine/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java

Line 1679 in 44a50de

assert shader.getId() > 0;
jmonkeyengine/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java

Line 1700 in 44a50de

if (shader.getId() == -1) {

pspeed42 · 2023-03-07T21:30:14Z

re: "While JME specifically reserves the value -1 for invalid IDs:"

...and that might actually be a valid ID also. Maybe JME should track invalid IDs in a different way.

stephengold · 2023-03-07T22:08:43Z

We could store IDs as long, which would open up many possibilities.

Sailsman63 · 2023-03-07T23:40:48Z

It appears that INVALID_ID is a semaphore for "Not connected to a native-side object". Not sure if there's a better way to track this. (It feels like something left over from a time when every byte in any object was at a premium.)

pspeed42 · 2023-03-08T01:44:44Z

Or Integer (capital 'i') so that it could be nullable.

neph1 · 2023-03-09T07:01:49Z

I think the tests in lines 105 and 132 should be specifically for == INVALID_ID instead of <= 0.

I will try this locally and see how it works both on linux and try to test it on mac as well. Thanks for looking into this.

neph1 · 2023-03-10T12:52:55Z

Got some more info from the modified build. App now loads but window is black. It seems the assertion was legit in this case:

java.lang.IllegalStateException: Some video driver error or programming error occurred. Framebuffer object status is invalid.
	at com.jme3.renderer.opengl.GLRenderer.checkFrameBufferError(GLRenderer.java:1838)
	at com.jme3.renderer.opengl.GLRenderer.updateFrameBuffer(GLRenderer.java:1996)
	at com.jme3.renderer.opengl.GLRenderer.setFrameBuffer(GLRenderer.java:2127)
	at com.jme3.renderer.RenderManager.renderViewPort(RenderManager.java:1189)
	at com.jme3.renderer.RenderManager.render(RenderManager.java:1287)
	at com.jme3.app.SimpleApplication.update(SimpleApplication.java:278)
	at com.jme3.system.lwjgl.LwjglWindow.runLoop(LwjglWindow.java:580)
	at com.jme3.system.lwjgl.LwjglWindow.run(LwjglWindow.java:669)
	at com.jme3.system.lwjgl.LwjglWindow.create(LwjglWindow.java:493)
	at com.jme3.app.LegacyApplication.start(LegacyApplication.java:490)
	at com.jme3.app.LegacyApplication.start(LegacyApplication.java:442)
	at com.jme3.app.SimpleApplication.start(SimpleApplication.java:126)

And then

SEVERE: Failed to destroy context
java.lang.IllegalArgumentException: The Image[size=1920x1080, format=Depth, id=0] NativeObject is not registered in this NativeObjectManager
	at com.jme3.util.NativeObjectManager.deleteNativeObject(NativeObjectManager.java:138)
	at com.jme3.util.NativeObjectManager.deleteUnused(NativeObjectManager.java:181)
	at com.jme3.util.NativeObjectManager.deleteAllObjects(NativeObjectManager.java:206)
	at com.jme3.renderer.opengl.GLRenderer.cleanup(GLRenderer.java:724)
	at com.jme3.system.lwjgl.LwjglWindow.destroyContext(LwjglWindow.java:442)
	at com.jme3.system.lwjgl.LwjglWindow.deinitInThread(LwjglWindow.java:646)
	at com.jme3.system.lwjgl.LwjglWindow.lambda$initInThread$12(LwjglWindow.java:518)
	at java.base/java.lang.Thread.dispatchUncaughtException(Unknown Source)

But I guess that is due to the first exception.
There's also a
WARNING: JmeDialogsFactory implementation not found.
Both for the first exception and the second. Presumably jme want's to show the exceptions in a dialog and fails.

The graphics card info makes me believe this is maybe related to Radeon cards, and not Macs:

INFO: OpenGL Renderer Information
 * Vendor: ATI Technologies Inc.
 * Renderer: AMD Radeon Pro Vega 20 OpenGL Engine
 * OpenGL Version: 4.1 ATI-4.8.101
 * GLSL Version: 4.10
 * Profile: Core

Would it be worth downgrading lwjgl to 2.9?

Ali-RS · 2023-03-10T13:38:56Z

There's also a
WARNING: JmeDialogsFactory implementation not found.
Both for the first exception and the second. Presumably jme want's to show the exceptions in a dialog and fails.

This warning can be resolved by adding the jme3-awt-dialogs module but also I believe swing + lwjgl3 is problematic on mac and I guess that is why awt dialogs were moved out in JME 3.6 in the first place.

The graphics card info makes me believe this is maybe related to Radeon cards, and not Macs:

Just in case, I am using an AMD ATI Radeon graphics card on Linux and do not have such issue.

neph1 · 2023-03-11T08:12:03Z

One thing I notice is that
@param waitFor true→wait for the context to be initialized,
is false in the start sequence. Could that be relevant perhaps in combination with the
-XstartOnMainThread
flag that is required for mac? Or just for some reason in general.

Ali-RS · 2023-03-11T08:54:49Z

I think the waitFor flag has no effect on mac

jmonkeyengine/jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglWindow.java

Lines 533 to 555 in a0c2c5e

    
           public void create(boolean waitFor) { 
        
               if (created.get()) { 
        
                   LOGGER.warning("create() called when display is already created!"); 
        
                   return; 
        
               } 
        
               if (Platform.get() == Platform.MACOSX) { 
        
                   // NOTE: this is required for Mac OS X! 
        
                   mainThread = Thread.currentThread(); 
        
                   mainThread.setName("jME3 Main"); 
        
                   if (waitFor) { 
        
                       LOGGER.warning("create(true) is not supported for macOS!"); 
        
                   } 
        
                   run(); 
        
               } else { 
        
                   mainThread = new Thread(this, "jME3 Main"); 
        
                   mainThread.start(); 
        
                   if (waitFor) { 
        
                       waitFor(true); 
        
                   } 
        
               } 
        
           }

stephengold · 2023-03-19T07:23:30Z

More evidence that JME requires native-object IDs to be non-negative:

jmonkeyengine/jme3-core/src/main/java/com/jme3/audio/AudioBuffer.java

Lines 133 to 136 in cef0d3b

    
           @Override 
        
           public long getUniqueId() { 
        
               return ((long) OBJTYPE_AUDIOBUFFER << 32) | ((long) id); 
        
           }

If the ID were negative, sign extension during the conversion to long would obliterate the object-type information.

andygibson · 2023-07-27T20:40:22Z

Might the answer here be encapsulation? Exposing the id and using it to indicate both the id of the object and the validity of it seems to be the root of the problem. Since -1 (or other negative numbers) can be a valid ID but also a signal for an invalid ID/Object, it is contradictory.

We can encapsulate the id, and provide separate methods to indicate object validity. By removing access to id, we are able to ensure we can capture and fix every case where the id is used or updated. Instead of checking for -1, we can check the isValid method. When we delete the object, we can call an invalidate() method to clear the id and the validity flag.

Behind the scenes, we can either use a separate boolean valid field, store it in the top half of a long id, or even still use -1. How it is implement can change once encapsulated, and tbh, the JVM will probably end up inlining those getter calls to direct variable accesses anyway.

I've put in PR: #2051 as a prototype for what I mean. Perhaps anyone who can reproduce the problem can see if it fixes it.

riccardobl · 2023-08-19T11:10:26Z

Encapsulation can be a breaking change for older code.
Since the issue is that the id is an uint that overflows when converted to signed int, I have a different proposal:

store the unsigned id in a long.
return the long from getId().
add setId(long).
deprecate setId(int) and replace its code to convert the overflowed int to an unsigned int .
warn when setId(-1) is called and ask the developer to use setId(INVALID_ID) instead.

Assuming most code uses setId(INVALID_ID) and not setId(-1), this shouldn't cause any issue, and if it doesn't then there is the warning that gives a chance to figure out that there is something wrong and how to fix it.

Pseudo code:

public static final **long** INVALID_ID=-1;
private **long** id;

public long getId(){
    return id;
}

@Deprecated
public void setId(int id){
    if(id==-1){
       warn("ID was set to -1. Note: this is a valid ID now, please use setId(INVALID_ID) to reset the id to the invalid ID");
    }
    this.id=toUnsignedInt(id);    
}

public void setId(**long** id){
    this.id=id;
}

Ali-RS added this to the Future Release milestone Mar 9, 2023

Ali-RS added the bug Something that is supposed to work, but doesn't. More severe than a "defect". label Mar 9, 2023

stephengold mentioned this issue Mar 30, 2023

correctly handle negative IDs in getUniqueId() methods #1991

Merged

andygibson linked a pull request Jul 27, 2023 that will close this issue

1982: Negative Native Object Id Issue #2051

Open

Ali-RS linked a pull request Jul 27, 2023 that will close this issue

1982: Negative Native Object Id Issue #2051

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IllegalArgumentException in registerObject() #1982

IllegalArgumentException in registerObject() #1982

stephengold commented Mar 7, 2023

stephengold commented Mar 7, 2023

pspeed42 commented Mar 7, 2023

stephengold commented Mar 7, 2023

Sailsman63 commented Mar 7, 2023

pspeed42 commented Mar 8, 2023

neph1 commented Mar 9, 2023

neph1 commented Mar 10, 2023

Ali-RS commented Mar 10, 2023 •

edited

Loading

neph1 commented Mar 11, 2023

Ali-RS commented Mar 11, 2023 •

edited

Loading

stephengold commented Mar 19, 2023

andygibson commented Jul 27, 2023 •

edited

Loading

riccardobl commented Aug 19, 2023 •

edited

Loading

IllegalArgumentException in registerObject() #1982

IllegalArgumentException in registerObject() #1982

Comments

stephengold commented Mar 7, 2023

stephengold commented Mar 7, 2023

pspeed42 commented Mar 7, 2023

stephengold commented Mar 7, 2023

Sailsman63 commented Mar 7, 2023

pspeed42 commented Mar 8, 2023

neph1 commented Mar 9, 2023

neph1 commented Mar 10, 2023

Ali-RS commented Mar 10, 2023 • edited Loading

neph1 commented Mar 11, 2023

Ali-RS commented Mar 11, 2023 • edited Loading

stephengold commented Mar 19, 2023

andygibson commented Jul 27, 2023 • edited Loading

riccardobl commented Aug 19, 2023 • edited Loading

Ali-RS commented Mar 10, 2023 •

edited

Loading

Ali-RS commented Mar 11, 2023 •

edited

Loading

andygibson commented Jul 27, 2023 •

edited

Loading

riccardobl commented Aug 19, 2023 •

edited

Loading