<Swing Dev> 7189299 DefaultButtonModel instance keeps stale listeners of JButton in case of multiple SwingUtilities.updateComponentTreeUI() calls

Alexander Scherbatiy alexandr.scherbatiy at oracle.com
Tue Nov 6 13:26:53 UTC 2012


On 11/5/2012 11:20 AM, Frank Ding wrote:
> Hi Alexander,
>   Based on your comments last time, I refined my patch of v6 and 
> offered v7 @
>   http://cr.openjdk.java.net/~dingxmin/7189299/webrev.07/

      This version of the fix looks good for me.
      It seems that it is the only good way to check that a button model 
contains AbstarctButton.Handler listener.

     Could you also check that others models  used in the  
createComponent() method do not have such memory leaks (even the NPE is 
not thrown)?

>   4.  Could you add the check that the action listener is invoked only 
> once after a component tree updating and the action does the same that 
> it does before a component tree updating?
>   Answer:  I am afraid I could not make it in the auto test 
> (bug7189299.java) but I can achieve it to some degree in manual test 
> FormSubmit, the one you illustrated below.
>   My idea of checking it in FormSubmit.java is subclassing JEditorPane 
> and overriding 'public EditorKit getEditorKit()' where stack traces 
> can be examined in the overridden method to make sure 
> FormView.submitData occurs only once.  This approach works because 
> FormView.submitData() calls JEditorPane.getEditorKit but is tricky.  
> However, it's the only way I can think of without any additional 
> framework support.  If you are in favor of adding the check in 
> FormSubmit.java, I am willing to do that.

       At least a separated manual test can be added that asks a user to 
put a response.html file to a server and according to the server url 
checks that JTeditor pane show the response text after a button pressing.

         html = new JEditorPane("text/html",
                 "<html><body><form action=\"" + userURL + "\">"
                         + "<input type=submit name=submit 
value=\"submit\"/>"
                         + "</form></body></html>");

  response.html:
<html> <body> Hello World! </body> </html>

   Thanks,
   Alexandr.

>   Thanks again for all your comments and your support.  Once again, if 
> you have any further concern or comment, please don't hesitate to let 
> me know.
>
> Best regards,
> Frank
>
>
> On 11/1/2012 8:08 PM, Alexander Scherbatiy wrote:
>> On 11/1/2012 10:35 AM, Frank Ding wrote:
>>> Hi Alexandr,
>>>   Thanks for your review.  The problem that the JButton does not 
>>> release itself as you mentioned is, after taking closer look at 
>>> relevant code again, that I removed instance AbstractButton.Handler 
>>> as change listener where repainting the button press animation is 
>>> defined.  So this time I get around this issue and here comes v6 
>>> patch @
>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.06/
>>>
>>>   Below are answers to what you have raised in previous email.
>>> 1. if ( if (!listener.equals(this)) { that 'this' means the 
>>> JButtonWrapper instead of an action listener.
>>>   'this' refers to JButtonWrapper.this because the closest class 
>>> definition is JButtonWrapper.  I also verified it during my debugging.
>>
>>     Could you give an example there this check is useful? It seems 
>> that the JButtonWrapper does not implement ActionListener interface 
>> and so the listener and the this always should not be equal.
>>
>>
>>    I dumped action listeners from the button and model before the fix 
>> and after it.
>>    Before the fix the button has the latest FormView listener and the 
>> model has AbstractButton$Handler added each time after a component 
>> input creation.
>>    After the fix both the button and the model have the FormView 
>> listener and the button does not have the AbstractButton$Handler.
>>    Could you preserve the listeners location and just remove old 
>> listeners from the model?
>>
>>    The fix sets the model to the JButtonWrapper first and than 
>> removes unneccessary listeners from the model. Is it possible to 
>> rearrange order of these operations?
>>
>>    I see that you check number of the listeners in the test. Could 
>> you add the check that the action listener is invoked only once after 
>> a component tree updating and
>>    the action does the same that it does before a component tree 
>> updating.
>>
>>    Thanks,
>>    Alexandr.
>>>
>>> 2. Why are two FormViews created in this sample?
>>> SwingUtilities.updateComponentTreeUI() triggers html View hierarchy 
>>> to update by instantiating new UI instances involved in the html 
>>> form.  It could have been avoided to have 2 View instances but we 
>>> are restricted to current design that LF change causes creation of 
>>> View instance which further leads to creation of new UI instances.  
>>> Creation of new UI instances upon LF change can be referenced in 
>>> BasicTextUI.modelChanged().
>>>
>>> 3. Why are there 2 listeners added to the button model in this sample?
>>> The issue is caused by combination of the 2 factors.  First, a 
>>> shared ButtonModel is created per html Element.  Second, 
>>> JButton.setModel() automatically registers its private Handler in 
>>> the new model but no way to deregister it.  The first is a design 
>>> choice but the second is a design flaw.  Consequence is that the 
>>> shared ButtonModel instance bears more and more JButton.Handler.
>>>
>>> I think v6 patch is promising as a final fix.  If you have any 
>>> further concern or comment, please don't hesitate to let me know. 
>>> Thanks again for your careful review.
>>>
>>> Best regards,
>>> Frank
>>>
>>> On 10/30/2012 9:38 PM, Alexander Scherbatiy wrote:
>>>> Hi Frank,
>>>>
>>>> I tried your V5 patch and run the test described in the issue (see 
>>>> below).
>>>> I pressed the button and it does not release. This is because the 
>>>> AbstractButton.Handler listener is removed because of the fix.
>>>> You could check the line
>>>>     941   if (!listener.equals(this)) {
>>>> that 'this' means the JButtonWrapper instead of an action listener.
>>>>
>>>> Could you also look deeper in the code and say why two FormViews 
>>>> are created (and each adds it's own listener to the button model) 
>>>> in this sample?
>>>>
>>>> -----------------------------------------------
>>>> import java.awt.*;
>>>> import javax.swing.*;
>>>>
>>>> public class FormSubmit {
>>>>
>>>>     public static void main(String[] args) throws Exception {
>>>>         SwingUtilities.invokeAndWait(new Runnable() {
>>>>
>>>>             @Override
>>>>             public void run() {
>>>>
>>>>                 JEditorPane html = new JEditorPane("text/html", 
>>>> "<html><body><FORM ACTION=\"examplescript.cgi\"><INPUT 
>>>> type=\"submit\" value=\"Submit\"></FORM></BODY></HTML>");
>>>>                 JFrame frame = new JFrame()
>>>>                 frame.setLayout(new BorderLayout());
>>>>                 frame.add(html, BorderLayout.CENTER);
>>>>                 frame.setSize(200, 100);
>>>> frame.setDefaultCloseOperation(frame.EXIT_ON_CLOSE);
>>>>                 frame.setVisible(true); // Uncomment this line to 
>>>> see the Exception on JDK 7
>>>>                 SwingUtilities.updateComponentTreeUI(html);
>>>>             }
>>>>         });
>>>>     }
>>>> }
>>>> -----------------------------------------------
>>>>
>>>> Thanks,
>>>> Alexandr.
>




More information about the swing-dev mailing list