[NbBuild 200302110100, jdk1.4.1]
At setCurrentNode() adds WeakListener but try to
remove original listener (lines 476, 497-499).
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
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
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.