Selecting "Close Window" leaves the app in a zombie state

[Robert Leach]

If you select Close Window from the File menu, a dialog appears asking if you're sure you want to exit treeview. Upon clicking yes, the wondow closes, but the app keeps running. Selecting anything from the menus after closing the window does nothing. So perhaps upon closing the window, the app should just quit itself, or else - don't provide a close window option in the menu. You can see the issue in the middle of this screencast that I made for the slow initial open issue:

http://gen-rlimac.princeton.edu/~rleach/TREEVIEWBUGS/slow-initial-open.mov

[Robert Leach]

It appears that this has been fixed. If anyone find that there's a use-case where it doesn't work, please re-open this issue.

[Anastasia Baryshnikova]

Tested now and the app still keeps running after closing the window (but nothing can be done, e.g., I can't load a new file).

[Robert Leach]

Yeah, I noticed that sometimes it shuts down, other times it doesn't. The shut-down code is in there. It just doesn't always (or usually) work. We'll have to look into this further. The method used for shutting down upon prefs reset seems reliable. Perhaps we can glean something from that code to help with this issue...

[Anastasia Baryshnikova]

Technically, closing the window should just close the window, without shutting down the whole app, right? To me the bug is in the fact that you can't do anything after closing the window. I'm about to add another (probably related) issue: File > Open or File > Open recent doesn't seem to work at any point (not just after closing the window), so maybe fixing that one will fix this issue too.

[Robert Leach]

I believe I have fixed this. I'll reopen if anyone finds a problem.

[Robert Leach]

@TreeView3Dev and I discussed the appropriateness of the method I used to resolve this bug (manipulation of the default action to EXIT_ON_CLOSE or DO_NOTHING_ON_CLOSE on the fly). It's not conventional. It might cause problems. Though initial testing hasn't shown any yet - they could accumulate. It would also be an issue if we decide to support multiple windows (though all existing frames can be counted to allow us to decide not to exit).

I tested DISPOSE_ON_CLOSE and was able to reproduce the original bug by simply opening a file and performing a search, following by a close & confirm exit. The app remains alive.

I left the dispose code in the source, but commented out. Everything to switch it back to dispose is in ViewFrame.java.

I'm going to document Chris & my's discussion by pasting relevant parts of it here:

*The setDefaultCloseOperation() method for JFrame should really be set only once right after the JFrame object is instantiated. That is done on the applications main JFrame "appFrame" in the ViewFrame class. The method just specifies what happens when the user hits the red x. It definitely needs to be DISPOSE_ON_CLOSE rather than EXIT_ON_CLOSE or DO_NOTHING_ON_CLOSE. The JVM will only fully terminate when all Window instances are disposed and no non-daemon threads are open.

The reason that the latter was implemented before is that in Java TreeView (2.0) you could have multiple windows open (I meant to only temporarily disable that because it used to be implemented in AWT rather than Swing which, when mixed, could potentially cause some nasty bugs). When you close window instances only you don't want the entire application to quit. Documentation: http://docs.oracle.com/javase/7/docs/api/javax/swing/JFrame.html#setDefaultCloseOperation(int)

Perhaps the [original] issue was that a try/catch was failing. I remember seeing a try catch. What if we set the exitonclose in the catch as a backup method? It wouldn't solve the slowness issue, but might be more reliable. I'd have to confirm that's what's happening, which might be hard to do.

I've been reading on this issue and it appears contentious and complex. Some threads I read suggest that EXIT_ON_CLOSE leaves leaks in the JVM. Others suggest that the JVM cleans up abandoned threads after exit and that the leaks issue is old/outdated. The documentation you linked me to says that EXIT_ON_CLOSE is OK for applications (assuming there are not multiple windows).

My testing shows that all the thread cleanup happens after dispose returns, so I don't think I'd be able to catch when the program is not going to terminate. I read some threads where people were having issues with dispose not ending up in the finalize code being called. Debugging that issue seems to be possible, but a difficult problem to account for all possible failures.

I noted that dispose is explicitly called in the windowClose method (even when EXIT_ON_CLOSE is set). I'm unclear whether that is interrupted when EXIT_ON_CLOSE is set. If we're calling dispose explicitly and we can account for other JFrames existing, will the disposal clean up the threads after the app exits (i.e. will the JVM keep going to do the cleanup)?

I know that EXIT_ON_CLOSE would be a problem if we introduced multiple windows, although I saw that there is a way to collect all existing JFrames to be checked/counted so that you could potentially opt to not exit. But if we don't use EXIT_ON_CLOSE, all the other ways of explicitly exiting seem to be more contentious than using EXIT_ON_CLOSE.

I even read one thread that mentioned cleaning up threads from previous executions on startup. Have you ever heard of that?

Suffice it to say, I am not versed enough in Java to know what the best thing to do is here. So I'm thinking of just setting DISPOSE_ON_CLOSE and re-open the ticket for you to eventually get to. Thoughts?

I'm going to spend a little time to see if I can get the dispose but no-exit scenario to happen.

Seems to be a very complex issue. I considered readding the possibility of opening multiple DendroView instances (in different windows).

Other than that I suppose for now we should go for whatever works and is somewhat reliable. Even if it is unconventional. As long as we don't use System.exit() we should be fine. Once there's some time available I can take a deeper look into this unless we don't have a reliable way of closing. That would be a high priority bug.*

[Robert Leach]

I commented out the call to System.exit and changed the default close behavior to do nothing on close (because the app was quitting even when selecting "No" or closing the confirmation dialog). See issue #396.