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.

Bug 30946

Summary: Property Sheet doesn't remove ActiveNodeListener(s)
Product: platform Reporter: Martin Schovanek <mschovanek>
Component: ExplorerAssignee: _ 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
[NbBuild 200302110100, jdk1.4.1]

At setCurrentNode() adds WeakListener but try to
remove original listener (lines 476, 497-499).
Comment 1 _ tboudreau 2003-02-11 15:43:21 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.
Comment 2 David Strupl 2003-02-11 15:51:31 UTC
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).
Comment 3 Martin Schovanek 2003-02-11 16:14:47 UTC
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.
Comment 4 _ tboudreau 2003-02-11 16:16:52 UTC
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?
Comment 5 David Strupl 2003-02-11 16:39:16 UTC
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).
Comment 6 _ tboudreau 2003-02-11 19:36:16 UTC
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...).
Comment 7 Martin Schovanek 2003-02-12 10:02:20 UTC
Okay. Tim - send me the patch please and i will test it.
Comment 8 _ tboudreau 2003-02-12 15:11:25 UTC
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.
Comment 9 Martin Schovanek 2003-02-25 11:20:02 UTC
*** Issue 31305 has been marked as a duplicate of this issue. ***
Comment 10 Marian Mirilovic 2003-04-08 16:20:07 UTC
Martin, please verify, thanks in advance.
Comment 11 Martin Schovanek 2003-04-08 16:34:03 UTC
VERIFIED