Hello,
I have not been very active in the JME community, but still i have been working with it a bit and can look back to 12 years of professional Java experience. I want to suggest a slight modification of StandardGame going together with a couple of new classes.
The problem has come up a few times. And i have have described it here.
svenpohl said:
As discussed in other threads before, there occur problems with the shutdown of StandardGame in a threaded environment. There are always at least the main thread and the OpenGL thread, so this should apply for nearly everybody.
The relevant threads:
* Program exit?
* How to cleanly shut down StandardGame ?
* Threading and StandardGame
The problem typically occurs when the OpenGL thread closes (f.e. because the user clicked the X of the window) but other threads still run and use GameTaskQueueManager with a .get() on the returned Future object. Try closing your games window when your main thread still set's up your GameStates ;)
For cases where no cleaning up of the setup thread is required, there is a simpler solution that has been proposed by Momoko_Fan:
final StandardGame game = new StandardGame("TestLock");
game.start();
// setup all the game states in daemon thread, to shutdown when OpenGL thread shuts
Thread setupThread = new Thread(new SetupProcess(game));
setupThread.setDaemon(true);
setupThread.start();
But if any running thread wants to clean up before dying...
...
I created a pretty ugly workaround then I tried to cleanly integrate my work into JME.
The idea is to:
1) make sure the GameTask.get calls will be interrupted when the game quits
2) enable clients of GameShutdownManager to be informend when game shuts down (topically some threads)
3) enable the developer to set a final shutdown task, that will execute after shutdown of StandardGame is complete
My code so far contains
* a new class com.jme.util.GameShutdownManager,
* modifications to the GameTask, GameTaskQueue and GameTaskQueueManager classes, and
* a modification to the cleanup method of StandardGame.
The code follows below. I use the latest SVN version.
EDIT: I created a diff patch for all. Should make things easier.
Index: D:/Development/workspace/jme/src/com/jmex/game/StandardGame.java
===================================================================
--- D:/Development/workspace/jme/src/com/jmex/game/StandardGame.java (revision 4289)
+++ D:/Development/workspace/jme/src/com/jmex/game/StandardGame.java (working copy)
@@ -57,6 +57,7 @@
import com.jme.system.PreferencesGameSettings;
import com.jme.system.dummy.DummySystemProvider;
import com.jme.system.jogl.JOGLSystemProvider;
+import com.jme.util.GameShutdownManager;
import com.jme.util.GameTaskQueue;
import com.jme.util.GameTaskQueueManager;
import com.jme.util.NanoTimer;
@@ -434,6 +435,8 @@
}
protected void cleanup() {
+ GameShutdownManager.getManager().shutdownStarted();
+ GameTaskQueueManager.getManager().setEnabled(false);
GameStateManager.getInstance().cleanup();
DisplaySystem.getDisplaySystem().getRenderer().cleanup();
@@ -444,6 +447,7 @@
if (AudioSystem.isCreated()) {
AudioSystem.getSystem().cleanup();
}
+ GameShutdownManager.getManager().shutdownComplete();
}
protected void quit() {
Index: D:/Development/workspace/jme/src/com/jme/util/GameShutdownManager.java
===================================================================
--- D:/Development/workspace/jme/src/com/jme/util/GameShutdownManager.java (revision 0)
+++ D:/Development/workspace/jme/src/com/jme/util/GameShutdownManager.java (revision 0)
@@ -0,0 +1,67 @@
+package com.jme.util;
+
+import java.util.Iterator;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.logging.Logger;
+
+/**
+ * This class manages tasks that are executed when <code>StandardGame</code>
+ * shuts down. At least the main thread which can be considered the
+ * initialization thread of a game could still run if the OpenGL
+ * thread stops. It is recommended to add a <code>ShutdownListener</code> to
+ * the <code>GameShutdownManager</code> for every game that uses the
+ * <code>GameTaskQueueManager</code> and calls a <code>get</code> method on
+ * the returned <code>Future</code> objects.
+ * <p>
+ * Please note, that an internal game state is created and attached to the
+ * <code>GameStateManager</code> if listeners are added. Also note that this
+ * class is thread safe.
+ *
+ * @author Sven Pohl
+ * @version $Revision$, $Date$
+ */
+public class GameShutdownManager {
+ private static final Logger logger = Logger
+ .getLogger(GameShutdownManager.class.getName());
+
+ private static final GameShutdownManager INSTANCE = new GameShutdownManager();
+
+ private ConcurrentLinkedQueue<Runnable> queue = new ConcurrentLinkedQueue<Runnable>();
+ private AtomicReference<Runnable> finalTask = new AtomicReference<Runnable>();
+
+ private GameShutdownManager() {
+ }
+
+ public static GameShutdownManager getManager() {
+ return INSTANCE;
+ }
+
+ public void addTask(Runnable task) {
+ queue.add(task);
+ }
+
+ public void removeTask(Runnable task) {
+ queue.remove(task);
+ }
+
+ public void setFinalTask(Runnable task) {
+ finalTask.set(task);
+ }
+
+ public void shutdownStarted() {
+ logger.info("shutdownStarted");
+ for (Iterator<Runnable> i = queue.iterator(); i.hasNext();) {
+ Runnable listener = i.next();
+ listener.run();
+ }
+ }
+
+ public void shutdownComplete() {
+ logger.info("shutdownComplete");
+ Runnable task = finalTask.get();
+ if (task != null)
+ finalTask.get().run();
+ }
+
+}
Index: D:/Development/workspace/jme/src/com/jme/util/GameTaskQueue.java
===================================================================
--- D:/Development/workspace/jme/src/com/jme/util/GameTaskQueue.java (revision 4289)
+++ D:/Development/workspace/jme/src/com/jme/util/GameTaskQueue.java (working copy)
@@ -54,6 +54,7 @@
private final ConcurrentLinkedQueue<GameTask<?>> queue = new ConcurrentLinkedQueue<GameTask<?>>();
private final AtomicBoolean executeAll = new AtomicBoolean();
+ private final AtomicBoolean enabled = new AtomicBoolean(true);
/**
* The state of this <code>GameTaskQueue</code> if it
@@ -90,7 +91,7 @@
* @return
*/
public <V> Future<V> enqueue(Callable<V> callable) {
- GameTask<V> task = new GameTask<V>(callable);
+ GameTask<V> task = new GameTask<V>(callable, enabled);
queue.add(task);
return task;
}
@@ -111,4 +112,34 @@
task.invoke();
} while ((executeAll.get()) && ((task = queue.poll()) != null));
}
+
+ /**
+ * This method cancels all queued tasks and prevents any new added tasks from
+ * being blocked.
+ * Call this method to assure clean shutdown of StandardGame.
+ * Otherwise waiting threads are locked since the game thread has shut down.
+ * An exception may be a transition from one StandardGame to another.
+ */
+ public void setEnabled(boolean value) {
+ enabled.set(value);
+ if (value) return;
+
+ // cancel all tasks if not enabled
+ GameTask<?> task = queue.poll();
+ do {
+ if (task == null)
+ return;
+ while (task.isCancelled()) {
+ task = queue.poll();
+ if (task == null)
+ return;
+ }
+ task.cancel(true);
+ } while ((executeAll.get()) && ((task = queue.poll()) != null));
+ }
+
+ public boolean isEnabled() {
+ return enabled.get();
+ }
+
}
Index: D:/Development/workspace/jme/src/com/jme/util/GameTask.java
===================================================================
--- D:/Development/workspace/jme/src/com/jme/util/GameTask.java (revision 4289)
+++ D:/Development/workspace/jme/src/com/jme/util/GameTask.java (working copy)
@@ -36,6 +36,7 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Level;
@@ -59,8 +60,11 @@
private final ReentrantLock stateLock = new ReentrantLock();
private final Condition finishedCondition = stateLock.newCondition();
- public GameTask(Callable<V> callable) {
+ private AtomicBoolean enabled;
+
+ public GameTask(Callable<V> callable, AtomicBoolean enabled) {
this.callable = callable;
+ this.enabled = enabled;
}
public boolean cancel(boolean mayInterruptIfRunning) {
@@ -83,6 +87,9 @@
public V get() throws InterruptedException, ExecutionException {
stateLock.lock();
try {
+ if (!enabled.get())
+ throw new InterruptedException("task queue disabled");
+
while (!isDone()) {
finishedCondition.await();
}
Index: D:/Development/workspace/jme/src/com/jme/util/GameTaskQueueManager.java
===================================================================
--- D:/Development/workspace/jme/src/com/jme/util/GameTaskQueueManager.java (revision 4289)
+++ D:/Development/workspace/jme/src/com/jme/util/GameTaskQueueManager.java (working copy)
@@ -32,6 +32,7 @@
package com.jme.util;
+import java.util.Iterator;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@@ -92,4 +93,20 @@
public <V> Future<V> render(Callable<V> callable) {
return getQueue(GameTaskQueue.RENDER).enqueue(callable);
}
+
+ /**
+ * This method cancels all queued tasks and prevents any new added tasks from
+ * being blocked.
+ * Call this method to assure clean shutdown of StandardGame.
+ * Otherwise waiting threads are locked since the game thread has shut down.
+ * An exception may be a transition from one StandardGame to another.
+ */
+ public void setEnabled(boolean value) {
+ Iterator<GameTaskQueue> i = managedQueues.values().iterator();
+ while (i.hasNext()) {
+ GameTaskQueue queue = i.next();
+ queue.setEnabled(value);
+ }
+ }
+
}
StandardGame:
I added 3 lines of code to the cleanup method, which is called only to shutdown the game.
protected void cleanup() {
// added these two lines
GameShutdownManager.getManager().shutdownStarted();
GameTaskQueueManager.getManager().setEnabled(false);
GameStateManager.getInstance().cleanup();
DisplaySystem.getDisplaySystem().getRenderer().cleanup();
TextureManager.doTextureCleanup();
TextureManager.clearCache();
JoystickInput.destroyIfInitalized();
if (AudioSystem.isCreated()) {
AudioSystem.getSystem().cleanup();
}
// and added this one
GameShutdownManager.getManager().shutdownComplete();
}
Comments on the modification:
1) GameShutdownManager.getManager().shutdownStarted();
The new class GameShutdownManager is called in the beginning of the startup process to enable user code that are not game states to shutdown cleanly. Typically these are threads.
Alternative: without this it is possible to register a game state instead that does not do anything in update and render, but only in cleanup. Little drawback of that alternative: it would be called every frame without any need.
2) GameTaskQueueManager.getManager().setEnabled(false);
The new method setEnabled of GameTaskQueueManager is called to interrupt all waiting threads and make sure later calls to GameTask.get() are interrupted, too. Warning: this modification forces users who start another StandardGame after shutdown, to modify there code. They must add a setEnabled(true) call, but this really is not standard scenario.
Alternative: register a game state as described in 1)
3) GameShutdownManager.getManager().shutdownComplete();
After shutdown has been done, this enables clients to do something after shutdown is complete. I don't see any drawbacks of this feature, and it is very simple and makes things very easy that are difficult without.
Alternative: implement this in StandardGame instead of calling GameShutdownManager
GameShutdownManager:
package com.jme.util;
import java.util.Iterator;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Logger;
/**
* This class manages tasks that are executed when <code>StandardGame</code>
* shuts down. At least the main thread which can be considered the
* initialization thread of a game could still run if the OpenGL
* thread stops. It is recommended to add a <code>ShutdownListener</code> to
* the <code>GameShutdownManager</code> for every game that uses the
* <code>GameTaskQueueManager</code> and calls a <code>get</code> method on
* the returned <code>Future</code> objects.
* <p>
* Please note, that an internal game state is created and attached to the
* <code>GameStateManager</code> if listeners are added. Also note that this
* class is thread safe.
*
* @author Sven Pohl
* @version $Revision$, $Date$
*/
public class GameShutdownManager {
private static final Logger logger = Logger
.getLogger(GameShutdownManager.class.getName());
private static final GameShutdownManager INSTANCE = new GameShutdownManager();
private ConcurrentLinkedQueue<Runnable> queue = new ConcurrentLinkedQueue<Runnable>();
private AtomicReference<Runnable> finalTask = new AtomicReference<Runnable>();
private GameShutdownManager() {
}
public static GameShutdownManager getManager() {
return INSTANCE;
}
public void addTask(Runnable task) {
queue.add(task);
}
public void removeTask(Runnable task) {
queue.remove(task);
}
public void setFinalTask(Runnable task) {
finalTask.set(task);
}
public void shutdownStarted() {
logger.info("shutdownStarted");
for (Iterator<Runnable> i = queue.iterator(); i.hasNext();) {
Runnable listener = i.next();
listener.run();
}
}
public void shutdownComplete() {
logger.info("shutdownComplete");
Runnable task = finalTask.get();
if (task != null)
finalTask.get().run();
}
}
Now we are coming to the GameTaskQueueManager, GameTaskQueue and GameTask classes.
The new setEnabled method connects to the cancel method that already existed in GameTask class. This interrupts all waiting threads. Before that an enabled flag is set on each queue to prevent later calls to GameTask.get from locking. This mechanism is also applicable for the supported custom game tasks that are not UPDATE or RENDER.
GameTaskQueueManager:
/**
* This method cancels all queued tasks and prevents any new added tasks from
* being blocked.
* Call this method to assure clean shutdown of StandardGame.
* Otherwise waiting threads are locked since the game thread has shut down.
* An exception may be a transition from one StandardGame to another.
*/
public void setEnabled(boolean value) {
Iterator<GameTaskQueue> i = managedQueues.values().iterator();
while (i.hasNext()) {
GameTaskQueue queue = i.next();
queue.setEnabled(value);
}
}
GameTaskQueue:
private final AtomicBoolean enabled = new AtomicBoolean(true);
/**
* This method cancels all queued tasks and prevents any new added tasks from
* being blocked.
* Call this method to assure clean shutdown of StandardGame.
* Otherwise waiting threads are locked since the game thread has shut down.
* An exception may be a transition from one StandardGame to another.
*/
public void setEnabled(boolean value) {
enabled.set(value);
if (value) return;
// cancel all tasks if not enabled
GameTask<?> task = queue.poll();
do {
if (task == null)
return;
while (task.isCancelled()) {
task = queue.poll();
if (task == null)
return;
}
task.cancel(true);
} while ((executeAll.get()) && ((task = queue.poll()) != null));
}
public boolean isEnabled() {
return enabled.get();
}
GameTask:
// added
private AtomicBoolean enabled;
// modified constructor to save the enabled reference
public GameTask(Callable<V> callable, AtomicBoolean enabled) {
this.callable = callable;
this.enabled = enabled;
}
// modified to check enabled flag
public V get() throws InterruptedException, ExecutionException {
stateLock.lock();
try {
// ADDED these two lines
if (!enabled.get())
throw new InterruptedException("task queue disabled");
while (!isDone()) {
finishedCondition.await();
}
if (exception != null) {
throw exception;
}
return result;
} finally {
stateLock.unlock();
}
}
Since I never got an answer to my previous posts (a year ago ;) ) I would appreciate an answer very much.
Thanks for the great engine! Don't think I only argue as a newbie. I have been working with it quite a bit now even since I have not been very active in this forum.