apache
Correctly register weak property change listener on InputOutput
This warning was observed:
WARNING [org.openide.util.WeakListenerImpl]: Can't remove java.beans.PropertyChangeListener using method org.netbeans.modules.terminal.ioprovider.TerminalInputOutput.removePropertyChangeListener from org.netbeans.modules.terminal.ioprovider.TerminalInputOutput@10efb8c9
WARNING [org.openide.util.WeakListenerImpl]: Can't remove java.beans.PropertyChangeListener using method org.netbeans.modules.terminal.ioprovider.TerminalInputOutput.removePropertyChangeListener from org.netbeans.modules.terminal.ioprovider.TerminalInputOutput@3a72784a
This was one of the candidates for the source of the problem.
Isn't this because the pcs and vcs in TerminalInputOutput are firing with the wrong source? No objection to fixing like this, but could be done without additional API.
Isn't this because the pcs and vcs in
TerminalInputOutputare firing with the wrong source? No objection to fixing like this, but could be done without additional API.
The source of the events is indeed the TerminalInputOutput, but the registration is not done on it. That is a capability implemented by the IONotifier implementation. This is hidden by the IONotifier#addPropertyChangeListener implementation. The problem is now, that to create a WeakListener you need access to the object you need to detach (in this case the IONotifier implementation) from when the listener is collected.
I though about directly implementing the registration logic (essentially copying the IONotifier#find method), but discarded that so that the logic is all unified in IONotifier.
The source of the events is indeed the TerminalInputOutput, but the registration is not done on it.
Which is the problem IMO. ie. the pcs() method construction code at https://github.com/apache/netbeans/blob/master/ide/terminal.nb/src/org/netbeans/modules/terminal/ioprovider/TerminalInputOutput.java#L151 should be passing in the instance of MyIONotifier not this.
I think it is even more ugly as I though. I'll have to revisit this later.
@neilcsmith-net I reworked this to not require API changes. I decided against changing the source of the events or the way of the intregration, so that the change does not affect behavior outside the target handler. For that handler I implemented a WeakListener myself, which is not as universal as the one from the openide package, but is tailored to the situation.
I intent to merge this early next week. If anyone wants to object, please do so now.
As I was reminded, that we are already freezing, lets get this in now.