This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Property Sheet doesn't remove ActiveNodeListener(s) | ||
---|---|---|---|
Product: | platform | Reporter: | Martin Schovanek <mschovanek> |
Component: | Explorer | Assignee: | _ tboudreau <tboudreau> |
Status: | VERIFIED FIXED | ||
Severity: | blocker | CC: | bht, dstrupl |
Priority: | P2 | ||
Version: | 3.x | ||
Hardware: | PC | ||
OS: | Linux | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 29866 |
Description
Martin Schovanek
2003-02-11 13:59:25 UTC
So this is a memory leak - or listener leak? Please clarify the implications of this bug - the property sheet rewrite will solve it; is there a need for an immediate fix in the old property sheet code? Probably PropertySheet should not use WeakListener at all - the selected node is well defined, and when it changes, it is easy to remove listeners from the previous selection. If the user leaves open a property sheet for some transient node, that node cannot be collected, but presumably that is correct behavior. The only possible problem I can see is for things like the Options dialog which can lurk in memory; the last selected node could be weakly referenced if really necessary. Cc'ing David Strupl, to clarify why WeakListener is needed. Martin is right. The current (wrong) situation came into existence like this: I used weak listener and did not dettach it, phrebejk tried to explicitly remove the listeners but probably made a typo here. So the answer to Tim's question is: yes it should be corrected in the current version (trunk, NB35). It's listener leak. It's problem in db module, each connect, disconnect on DB increase number of ActiveNodeListener(s) by a factor of two. And when you several times connect and disconnect DB than connect and disconect time is up to 10 minutes. See #29866. Okay. David - what was using WeakListener solving? Can I replace it with a normal PropertyChangeListener that should be easy to remove, or will that cause a leak somewhere else? I don't remember exactly. It might be the case I was just to lazy to find the places where to remove the listener. Please note that when the listener is strong (not weak) the node references the prop. sheet even in case the prop. sheet is not displayed. So using the weak listener probably made sure that the prop. sheet itself is not referenced when no longer needed. As to explicitly removing the strong listener - you must also deregister the listener when hiding the prop. sheet (removeNotify?) and attach it when showing it. I am not sure about this. Has to be further examined. I propose to test the case I have described like this: try to show prop sheet for a node, hide (dispose) the prop. sheet, garbage collect and watch whether the sheet is still referenced (while still holding a ref to the node itself). Sounds like I can probably use a strong listener and check isVisible() on the prop sheet, if so, attach it, if not, flag it for add/remove on addNotify()/removeNotify() and it should work cleanly. Martin, I don't have any databases installed on my machine. If I send you a patch tomorrow, will you work with me to make sure it is really fixed? It should be simple, but there are probably some weird corner cases where the listener could remain attached (if anything sets the selected node off the AWT thread while something else is also setting the selected node, things like that...shouldn't happen but could...). Okay. Tim - send me the patch please and i will test it. Okay, the problem was that a weak listener was created to wrap the real NodeListener, but the code attempted to remove the NodeListener, not the weak listener wrapper. It looks like it is working correctly now, so I've committed the fix to the trunk, PropertySheet.java 1.103 Reopen if you still have the problem. *** Issue 31305 has been marked as a duplicate of this issue. *** Martin, please verify, thanks in advance. VERIFIED |