TestAwtPanels gives a NullPointerException

I tried running the TestAwtPanels app on my 64-bit win7 desktop, and it immediately hit an NPE. Here is the stack trace:

Apr 04, 2014 3:22:15 PM com.jme3.app.Application handleError
SEVERE: Uncaught exception thrown in Thread[LWJGL Renderer Thread,6,main]
java.lang.NullPointerException
	at jme3test.awt.TestAwtPanels.simpleInitApp(TestAwtPanels.java:84)
	at com.jme3.app.SimpleApplication.initialize(SimpleApplication.java:226)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:130)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:207)
	at java.lang.Thread.run(Thread.java:744)

It looks to me like there’s a race condition between the Runnable and simpleInitApp().

If i remember correctly I have hit a similar problem, caused by different working Os logic, apparently some do block till creation nativly others not.

My solution was to block till ready,as else the canvas might be invalid/not existent

[java]
ConfigFrame.app = new CompileApplication();
ConfigFrame.app.setShowSettings(false);
ConfigFrame.app.start(JmeContext.Type.Canvas);

	while (!ConfigFrame.app.started) { //set to true in the end of simple init
		try {
			Thread.sleep(100);
		} catch (final InterruptedException e) {
			e.printStackTrace();
		}
	}

	this.jcan = (JmeCanvasContext) ConfigFrame.app.getContext();

[/java]

Is busy waiting the best solution?

@Momoko_Fan should have a look at this as he implemented the current workarounds for the funny lwjgl/awt issues.

1 Like

Here’s a proposed fix to TestAwtPanels.java:

--- Base (BASE)
+++ Locally Modified (Based On LOCAL)
@@ -21,6 +21,7 @@
 
 public class TestAwtPanels extends SimpleApplication {
 
+    final private static Object panelsAreReady = new Object();
     private static TestAwtPanels app;
     private static AwtPanel panel, panel2;
     private static int panelsClosed = 0;
@@ -56,6 +57,16 @@
         
         SwingUtilities.invokeLater(new Runnable(){
             public void run(){
+                /*
+                 * Sleep 2 seconds to ensure there's no race condition.
+                 * The sleep is not required for correctness.
+                 */
+                try {
+                    Thread.sleep(2000);
+                } catch (InterruptedException exception) {
+                    return;
+                }
+
                 final AwtPanelsContext ctx = (AwtPanelsContext) app.getContext();
                 panel = ctx.createPanel(PaintMode.Accelerated);
                 panel.setPreferredSize(new Dimension(400, 300));
@@ -66,6 +77,12 @@
                 
                 createWindowForPanel(panel, 300);
                 createWindowForPanel(panel2, 700);
+                /*
+                 * Both panels are ready.
+                 */
+                synchronized (panelsAreReady) {
+                    panelsAreReady.notify();
+                }
             }
         });
     }
@@ -80,7 +97,17 @@
         mat.setTexture("ColorMap", assetManager.loadTexture("Interface/Logo/Monkey.jpg"));
         geom.setMaterial(mat);
         rootNode.attachChild(geom);
-        
+        /*
+         * Wait until both AWT panels are ready.
+         */
+        synchronized (panelsAreReady) {
+            try {
+                panelsAreReady.wait();
+            } catch (InterruptedException exception) {
+                return;
+            }
+        }
+
\ No newline at end of file
         panel.attachTo(true, viewPort);
         guiViewPort.setClearFlags(true, true, true);
         panel2.attachTo(false, guiViewPort);

Or if you use CountDownLatch you could reduce it to maybe fewer lines of code. This is essentially reimplementing most of countdown latch by hand and is a good example of a bad example, in my opinion… along the lines of trying to encourage people to use the proper threading primitives instead of rolling their own with synchronized. This is the opposite.

…though unfortunately you will still have to catch InterruptedException (stupid checked exceptions :)).

This:
final private static Object panelsAreReady = new Object();

Becomes:
final private static CountDownLatch panelsAreReady = new CountDownLatch(1);

This:
[java]
synchronized (panelsAreReady) {
panelsAreReady.notify();
}
[/java]

Becomes:
[java]
panelsAreReady.countDown();
[/java]

And this:
[java]
synchronized (panelsAreReady) {
try {
panelsAreReady.wait();
} catch (InterruptedException exception) {
return;
}
}
[/java]

Becomes:
[java]
try {
panelsAreReady.await();
} catch (InterruptedException exception) {
return;
}
[/java]

…but it’s instantly clearer what is going on without reading everywhere.

Generally, I’d throw a RuntimeException in the InterruptedException block. It’s the only appropriate response… which is kind of why InterruptedException being a checked exception is a little silly but that’s been decided a long time ago. :slight_smile:

throw new RuntimeException(“Interrupted waiting for panels”, e);

I’m fine with Paul’s suggestion, which saves 4 lines of code. I will re-test and open a new thread at the Contribution Depot

The latest changes allow TestAwlPanels to run fine as a standalone test. However, the test hangs when run from the TestChooser. I don’t know why.

Aha! TestChooser invokes TestAwtPanels.start() instead of TestAwtPanels.main(), so among other things the Runnable never gets created. I’m not sure why TestChooser was written that way, but in this instance it looks like a poor choice.

If I force the chooser to invoke TestAwtPanels.main(), the test seems to work.

I’ve contributed a fix for the race condition: http://hub.jmonkeyengine.org/forum/topic/fix-for-race-in-testawtpanels/