Cleanup() in AppState not called

Hello,

I followed the advise to prefer cleanup() over stateDetached() but I noticed that it is not called :expressionless: Can you tell me what I’m missing? I looked into the AppStateManager’s source but I cannot guess from it what I might do wrong.



Here an example:

[java]import com.jme3.app.SimpleApplication;



public class Main extends SimpleApplication {



@Override

public void simpleInitApp() {

stateManager.attach(new TestState());

}



@Override

public void simpleUpdate(float tpf) {

super.simpleUpdate(tpf);

if(stateManager.getState(TestState.class) != null) {

stateManager.detach(stateManager.getState(TestState.class));

System.out.println(“Done”); //Called

}

}



public static void main(String[] args) {

new Main().start();

}

}[/java]



[java]import com.jme3.app.state.AbstractAppState;

import com.jme3.app.state.AppStateManager;



public class TestState extends AbstractAppState {



@Override

public void stateDetached(AppStateManager stateManager) {

super.stateDetached(stateManager);

System.out.println(“Detached”); //Called

}



@Override

public void cleanup() {

super.cleanup();

System.out.println(“Cleanup”); //Not called

}



}[/java]

From the code it looks like your app state is never initialized. cleanup() is only called if initialize() has been called.



cleanup() is a chance for you to undo the things you did in initialize()… so it doesn’t make sense to call it if the state has never been initialize()ed.

Hmmm… no… I’m not sure I can explain it. I thought simpleUpdate() was called before stateManager.update() but I’m wrong.



Let me dig a little.

Ok, I first guessed that it might only be called when it “isInitialized()” but the AppStateManager-Code and this test try did proof different:



[java]import com.jme3.app.Application;

import com.jme3.app.state.AbstractAppState;

import com.jme3.app.state.AppStateManager;



public class TestState extends AbstractAppState {



boolean isInit = false;



@Override

public void stateDetached(AppStateManager stateManager) {

super.stateDetached(stateManager);

System.out.println(“Detached”);

}



@Override

public void initialize(AppStateManager stateManager, Application app) {

// TODO Auto-generated method stub

super.initialize(stateManager, app);

isInit = true;

}



@Override

public boolean isInitialized() {

// TODO Auto-generated method stub

return isInit;

}



@Override

public void cleanup() {

super.cleanup();

System.out.println(“Cleanup”);

}



}[/java]



I’m interested in your findings :wink:

Ok, I tested this… I even added a test for it into JME tests:

http://code.google.com/p/jmonkeyengine/source/browse/trunk/engine/src/test/jme3test/app/TestAppStateLifeCycle.java



When I run that test, I see:

[java]

Attaching test state.

Attached

Initialized

update

Detaching test state.

Detached

Done

Cleanup

[/java]



Which is exactly what I would have expected.



Maybe there is some subtle difference in the way our tests work or there is a version difference?



The latest stable (not nightly) should have the appropriate behavior, I think. Nothing has changed since then. At least I’m 99.9% sure that my app state changes went into a stable update.

Maybe there is some subtle difference in the way our tests work or there is a version difference?


I used the nightly from 20.02 and changed after noticing this behavior to 25.02...

I copied your test cases and this is what comes as output:
Attaching test state.
Attached
Initialized
update
Detaching test state.
Detached
Done


PS: If this leads to changes to the appstatemanager I would additionally request adding a method like the following that returns all AppStates that are subclass of a given class.
[java]public <T extends AppState> List<T> getStatesExtending(Class<? extends T> givenClass) {[/java]

Example where it will provide useful:
You write a server and some states manage things for every single user. And all these states need to be notified about a user connecting or leaving. If these states implement or extend a corresponding interface/class you could easily notify them without doing error prone lists like these:
app.getStateManager().getState(UserDependentState1.class).userRemoved(u);
app.getStateManager().getState(UserDependentState2.class).userRemoved(u);
@enum said:
I used the nightly from 20.02 and changed after noticing this behavior to 25.02...


I don't think anything changed about these classes between those two dates. I modified SimpleApplication to move most of what should be optionally out to separate states but that didn't change the state life cycle at all.

@enum said:
I copied your test cases and this is what comes as output:


And you didn't modify the test in any way? The clean is called on the following update pass... so if you've killed that application at the same time you remove that state then you'll never see the clean.

Actually, you should still see the clean since all app states get cleaned when the app shuts down if it shuts down properly.

@enum said:
Example where it will provide useful:
You write a server and some states manage things for every single user. And all these states need to be notified about a user connecting or leaving. If these states implement or extend a corresponding interface/class you could easily notify them without doing error prone lists like these:
app.getStateManager().getState(UserDependentState1.class).userRemoved(u);
app.getStateManager().getState(UserDependentState2.class).userRemoved(u);


Personally, I would never do this that way. In SpiderMonkey you can have all kinds of session attributes associated with the user. Creating an app state per user is not really the way app states were designed to work. (In fact, I generally find them useless on a server because there are geared towards a rendering pipeline... but that's another story.) In Mythruna, I just run over all of the connections, grab the attribute I want and call its update method. This set of things is guaranteed to only be the same as the list of active users because the connections go away when the user does.

I think it's better than having to allocate a new list every time you want a subset of app states. Adding that support might encourage bad behavior.

Even if you did want to use app states then you'd have one app state that had a list of all "things for the user". If you have more than 2 or 3 of a certain class of app state then I think you aren't really using them as intended.
1 Like
Creating an app state per user is not really the way app states were designed to work.


Thats not the way. I meant for example the TerrainManager which has one loader per user. When a new user connects, TerrainManager's addUser method is called and the terrainManager creates a new loader for that user (this is - as far as I can imagin - neccessary because two users may be in totally different parts of the world and therefore every user needs a loader to load the local world around it)
But I will consider what you said about the connections.

I really apprechiate that you share your knowledge. You are the person I learned most from while reading in this forum ;)

I did not do any change except of putting the two classes in two different files...
In fact the cleanup method IS called if one state is attached when the app is called to stop... But not when the state is just detached..
Today I have no time to think about it anymore... Tomorrow I will go on^^

Yeah, the terrain loader for that user is still user-specific. I personally wouldn’t do it as an app state since it’s not global (app states are by definition global.)



In Mythruna, I didn’t even do it this way anyway. The terrain has a central LRU cache and connections request terrain from the cache (and it’s loaded into cache if it doesn’t exist). I can control the size of the cache this way so that it doesn’t consume all memory. If I have 100 users on then they might get slow response from lots of cache misses but it doesn’t blow up the server. Of course, in my case, loading from file is only a few milliseconds anyway. For different scenarios, it might be better to have per-user cache/loaders or whatever and just limit the max users.



…but I still wouldn’t make them app states. :slight_smile:

1 Like

To Cleanup: I now attached the state in simpleUpdate and removed it in the next update cycle and still the cleanup method is not called… I even downloaded the source with SVN to look into the most recent code of SimpleApplication and AppStateManager and cannot find out why this behavior occurs… Did you test it with one of the nightlys?



Another thanks for the tip. I decided which chunks to load on the server… letting the clients request them seems better on the first glance but I am a little unsure about the AI System… The chunks around a user are really neccessary for that. (I had a combination of both in mind: One big cache but loader for every user which requests chunks from the chunkCache)

@enum said:
To Cleanup: I now attached the state in simpleUpdate and removed it in the next update cycle and still the cleanup method is not called.. I even downloaded the source with SVN to look into the most recent code of SimpleApplication and AppStateManager and cannot find out why this behavior occurs... Did you test it with one of the nightlys?


I tested again SVN.