Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-12 Thread Alexandr Scherbatiy 

The fix looks good to me.

Thanks,
Alexandr.

On 7/12/2016 3:58 PM, Andrej Golovnin wrote:

Hi Ajit,

it looks good for me. Thanks!
And you need a reviewer from the Swing team
as I don’t have the reviewer role.

Best regards,
Andrej Golovnin


-Original Message-
From: Ajit Ghaisas
Sent: Friday, July 08, 2016 10:32 AM
To: Andrej Golovnin
Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: RE: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

Hi Andrej,

 Thanks for your suggestion.

  I have made the 'updateInProgress' member of these classes transient.
  This is out of the fact that - 'updateInProgress' member is just an 
internal field of the class that need not be preserved during serialization.

   Here is the updated webrev. Request you to review.
   http://cr.openjdk.java.net/~aghaisas/6567433/webrev.03/

Regards,
Ajit



-Original Message-
From: Andrej Golovnin [mailto:andrej.golov...@gmail.com]
Sent: Thursday, July 07, 2016 4:44 PM
To: Ajit Ghaisas
Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

Hi Ajit,

one more thing that I have just noticed:

/**
  * Flag to indicate UI update is in progress
  */
private boolean updateInProgress;

I think the field must be transient. In Swing every component is serializable. 
When updateInProgress is set to true and you serialize/deserialize the 
component, then the call of the #updateUI()-method on the deserialized instance 
would never update the UI of the deserialized component because the flag 
updateInProgress will never change from true to false.

Best regards,
Andrej Golovnin

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-12 Thread Andrej Golovnin 
Hi Ajit,

it looks good for me. Thanks!
And you need a reviewer from the Swing team
as I don’t have the reviewer role.

Best regards,
Andrej Golovnin

> 
> -Original Message-
> From: Ajit Ghaisas 
> Sent: Friday, July 08, 2016 10:32 AM
> To: Andrej Golovnin
> Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: RE: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
> StackOverflowError
> 
> Hi Andrej,
> 
> Thanks for your suggestion.
> 
>  I have made the 'updateInProgress' member of these classes transient. 
>  This is out of the fact that - 'updateInProgress' member is just an 
> internal field of the class that need not be preserved during serialization.
> 
>   Here is the updated webrev. Request you to review.
>   http://cr.openjdk.java.net/~aghaisas/6567433/webrev.03/
> 
> Regards,
> Ajit
> 
> 
> 
> -Original Message-
> From: Andrej Golovnin [mailto:andrej.golov...@gmail.com] 
> Sent: Thursday, July 07, 2016 4:44 PM
> To: Ajit Ghaisas
> Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
> StackOverflowError
> 
> Hi Ajit,
> 
> one more thing that I have just noticed:
> 
> /**
>  * Flag to indicate UI update is in progress
>  */
> private boolean updateInProgress;
> 
> I think the field must be transient. In Swing every component is 
> serializable. When updateInProgress is set to true and you 
> serialize/deserialize the component, then the call of the #updateUI()-method 
> on the deserialized instance would never update the UI of the deserialized 
> component because the flag updateInProgress will never change from true to 
> false.
> 
> Best regards,
> Andrej Golovnin

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-12 Thread Ajit Ghaisas 
Hi Andrej,

Can you please review this?

Regards,
Ajit

-Original Message-
From: Ajit Ghaisas 
Sent: Friday, July 08, 2016 10:32 AM
To: Andrej Golovnin
Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: RE: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

Hi Andrej,

 Thanks for your suggestion.

  I have made the 'updateInProgress' member of these classes transient. 
  This is out of the fact that - 'updateInProgress' member is just an 
internal field of the class that need not be preserved during serialization.

   Here is the updated webrev. Request you to review.
   http://cr.openjdk.java.net/~aghaisas/6567433/webrev.03/
  
Regards,
Ajit
  
 

-Original Message-
From: Andrej Golovnin [mailto:andrej.golov...@gmail.com] 
Sent: Thursday, July 07, 2016 4:44 PM
To: Ajit Ghaisas
Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

Hi Ajit,

one more thing that I have just noticed:

 /**
  * Flag to indicate UI update is in progress
  */
 private boolean updateInProgress;

I think the field must be transient. In Swing every component is serializable. 
When updateInProgress is set to true and you serialize/deserialize the 
component, then the call of the #updateUI()-method on the deserialized instance 
would never update the UI of the deserialized component because the flag 
updateInProgress will never change from true to false.

Best regards,
Andrej Golovnin

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-07 Thread Ajit Ghaisas 
Hi Andrej,

 Thanks for your suggestion.

  I have made the 'updateInProgress' member of these classes transient. 
  This is out of the fact that - 'updateInProgress' member is just an 
internal field of the class that need not be preserved during serialization.

   Here is the updated webrev. Request you to review.
   http://cr.openjdk.java.net/~aghaisas/6567433/webrev.03/
  
Regards,
Ajit
  
 

-Original Message-
From: Andrej Golovnin [mailto:andrej.golov...@gmail.com] 
Sent: Thursday, July 07, 2016 4:44 PM
To: Ajit Ghaisas
Cc: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

Hi Ajit,

one more thing that I have just noticed:

 /**
  * Flag to indicate UI update is in progress
  */
 private boolean updateInProgress;

I think the field must be transient. In Swing every component is serializable. 
When updateInProgress is set to true and you serialize/deserialize the 
component, then the call of the #updateUI()-method on the deserialized instance 
would never update the UI of the deserialized component because the flag 
updateInProgress will never change from true to false.

Best regards,
Andrej Golovnin

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-07 Thread Andrej Golovnin 
Hi Ajit,

one more thing that I have just noticed:

 /**
  * Flag to indicate UI update is in progress
  */
 private boolean updateInProgress;

I think the field must be transient. In Swing every component is
serializable. When updateInProgress is set to true and you
serialize/deserialize the component, then the call of the
#updateUI()-method on the deserialized instance would never update the
UI of the deserialized component because the flag updateInProgress
will never change from true to false.

Best regards,
Andrej Golovnin

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-07 Thread Ajit Ghaisas 
Hi,

 

Thanks Rajeev and Andrej for the suggestions.

I have incorporated them in following webrev.

 

http://cr.openjdk.java.net/~aghaisas/6567433/webrev.02/

 

Regards,

Ajit

 

From: Rajeev Chamyal 
Sent: Thursday, July 07, 2016 3:30 PM
To: Alexander Scherbatiy; Ajit Ghaisas; swing-dev@openjdk.java.net
Subject: RE: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

Hello Ajit,

 

The fix looks fine to me.

Regarding test: JTable and JTree tests exceed 80 char limit.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 07 July 2016 15:17
To: Ajit Ghaisas; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

The fix looks good to me.

Thanks,
Alexandr.

On 7/7/2016 12:44 PM, Ajit Ghaisas wrote:

Hi,

  

Thanks Alex for pointing out there might be more components showing similar 
behavior.

 

Two more components are identified which may cause this recursion in 
UpdateUI() method - JTree and JTable.

 

Now, total 5 components ( JComboBox, JList, JTree, JTable and JTableHeader) 
are fixed.

 

Please review the updated webrev :

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.01/"http://cr.openjdk.java.net/~aghaisas/6567433/webrev.01/

 

Regards,

Ajit

 

From: Alexandr Scherbatiy 
Sent: Tuesday, July 05, 2016 5:55 PM
To: Ajit Ghaisas; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

On 7/5/2016 2:51 PM, Ajit Ghaisas wrote:



Hi,

 

Bug :

 https://bugs.openjdk.java.net/browse/JDK-6567433

 

Calling updateUI() on JList, JComboBox and JTableHeader can create 
StackOverflowErrors.
For example -

JList.updateUI() invokes updateUI() on its Cellrenderer via 
SwingUtilities.updateComponentTreeUI().
If the cellrenderer is a parent of this JList the method recurses endless 
causing StackOverflowError.

 

 

Fix :

 Added a recursion guard to JComboBox, JList and JTableHeader classes.

 With this fix, UpdateUI() method in these classes does not result in 
recursion.

 

Webrev :

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.00/"http://cr.openjdk.java.net/~aghaisas/6567433/webrev.00/

 
   Could the same issue affect another Swing components which allow to set a 
cell renderer, like JTree?

  Thanks,
  Alexandr.



 

Request you to review.

 

Regards,

Ajit

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-07 Thread Andrej Golovnin 
Hi Ajit,

src/java.desktop/share/classes/javax/swing/JList.java

In the line 545 there are unnecessary brackets:

545 if ((renderer instanceof Component)) {

It should look like this:

545 if (renderer instanceof Component) {

Best regards,
Andrej Golovnin

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-07 Thread Rajeev Chamyal 
Hello Ajit,

 

The fix looks fine to me.

Regarding test: JTable and JTree tests exceed 80 char limit.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 07 July 2016 15:17
To: Ajit Ghaisas; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

The fix looks good to me.

Thanks,
Alexandr.

On 7/7/2016 12:44 PM, Ajit Ghaisas wrote:

Hi,

  

Thanks Alex for pointing out there might be more components showing similar 
behavior.

 

Two more components are identified which may cause this recursion in 
UpdateUI() method - JTree and JTable.

 

Now, total 5 components ( JComboBox, JList, JTree, JTable and JTableHeader) 
are fixed.

 

Please review the updated webrev :

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.01/"http://cr.openjdk.java.net/~aghaisas/6567433/webrev.01/

 

Regards,

Ajit

 

From: Alexandr Scherbatiy 
Sent: Tuesday, July 05, 2016 5:55 PM
To: Ajit Ghaisas; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

On 7/5/2016 2:51 PM, Ajit Ghaisas wrote:




Hi,

 

Bug :

 https://bugs.openjdk.java.net/browse/JDK-6567433

 

Calling updateUI() on JList, JComboBox and JTableHeader can create 
StackOverflowErrors.
For example -

JList.updateUI() invokes updateUI() on its Cellrenderer via 
SwingUtilities.updateComponentTreeUI().
If the cellrenderer is a parent of this JList the method recurses endless 
causing StackOverflowError.

 

 

Fix :

 Added a recursion guard to JComboBox, JList and JTableHeader classes.

 With this fix, UpdateUI() method in these classes does not result in 
recursion.

 

Webrev :

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.00/"http://cr.openjdk.java.net/~aghaisas/6567433/webrev.00/

 
   Could the same issue affect another Swing components which allow to set a 
cell renderer, like JTree?

  Thanks,
  Alexandr.




 

Request you to review.

 

Regards,

Ajit

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-07 Thread Alexandr Scherbatiy 

The fix looks good to me.

Thanks,
Alexandr.

On 7/7/2016 12:44 PM, Ajit Ghaisas wrote:


Hi,

Thanks Alex for pointing out there might be more components 
showing similar behavior.


Two more components are identified which may cause this recursion 
in UpdateUI() method – JTree and JTable.


Now, total 5 components ( JComboBox, JList, JTree, JTable and 
JTableHeader) are fixed.


Please review the updated webrev :

http://cr.openjdk.java.net/~aghaisas/6567433/webrev.01/ 
<http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.01/>


Regards,

Ajit

*From:*Alexandr Scherbatiy
*Sent:* Tuesday, July 05, 2016 5:55 PM
*To:* Ajit Ghaisas; Rajeev Chamyal; swing-dev@openjdk.java.net
*Subject:* Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may 
create StackOverflowError


On 7/5/2016 2:51 PM, Ajit Ghaisas wrote:

Hi,

Bug :

https://bugs.openjdk.java.net/browse/JDK-6567433

Calling updateUI() on JList, JComboBox and JTableHeader can
create StackOverflowErrors.
For example -

JList.updateUI() invokes updateUI() on its Cellrenderer via
SwingUtilities.updateComponentTreeUI().
If the cellrenderer is a parent of this JList the method
recurses endless causing StackOverflowError.

Fix :

 Added a recursion guard to JComboBox, JList and JTableHeader
classes.

 With this fix, UpdateUI() method in these classes does not
result in recursion.

Webrev :

http://cr.openjdk.java.net/~aghaisas/6567433/webrev.00/
<http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.00/>


   Could the same issue affect another Swing components which allow to 
set a cell renderer, like JTree?


  Thanks,
  Alexandr.

Request you to review.

Regards,

Ajit

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-07 Thread Ajit Ghaisas 
Hi,

  

Thanks Alex for pointing out there might be more components showing similar 
behavior.

 

Two more components are identified which may cause this recursion in 
UpdateUI() method - JTree and JTable.

 

Now, total 5 components ( JComboBox, JList, JTree, JTable and JTableHeader) 
are fixed.

 

Please review the updated webrev :

http://cr.openjdk.java.net/~aghaisas/6567433/webrev.01/

 

Regards,

Ajit

 

From: Alexandr Scherbatiy 
Sent: Tuesday, July 05, 2016 5:55 PM
To: Ajit Ghaisas; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

On 7/5/2016 2:51 PM, Ajit Ghaisas wrote:



Hi,

 

Bug :

 https://bugs.openjdk.java.net/browse/JDK-6567433

 

Calling updateUI() on JList, JComboBox and JTableHeader can create 
StackOverflowErrors.
For example -

JList.updateUI() invokes updateUI() on its Cellrenderer via 
SwingUtilities.updateComponentTreeUI().
If the cellrenderer is a parent of this JList the method recurses endless 
causing StackOverflowError.

 

 

Fix :

 Added a recursion guard to JComboBox, JList and JTableHeader classes.

 With this fix, UpdateUI() method in these classes does not result in 
recursion.

 

Webrev :

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.00/"http://cr.openjdk.java.net/~aghaisas/6567433/webrev.00/

 
   Could the same issue affect another Swing components which allow to set a 
cell renderer, like JTree?

  Thanks,
  Alexandr.



 

Request you to review.

 

Regards,

Ajit

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-05 Thread Alexandr Scherbatiy 

On 7/5/2016 2:51 PM, Ajit Ghaisas wrote:


Hi,

Bug :

https://bugs.openjdk.java.net/browse/JDK-6567433

Calling updateUI() on JList, JComboBox and JTableHeader can create 
StackOverflowErrors.

For example -

JList.updateUI() invokes updateUI() on its Cellrenderer via 
SwingUtilities.updateComponentTreeUI().
If the cellrenderer is a parent of this JList the method recurses 
endless causing StackOverflowError.


Fix :

 Added a recursion guard to JComboBox, JList and JTableHeader classes.

 With this fix, UpdateUI() method in these classes does not result 
in recursion.


Webrev :

http://cr.openjdk.java.net/~aghaisas/6567433/webrev.00/ 





   Could the same issue affect another Swing components which allow to 
set a cell renderer, like JTree?


  Thanks,
  Alexandr.


Request you to review.

Regards,

Ajit

[9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

2016-07-05 Thread Ajit Ghaisas 
Hi,

 

Bug :

 https://bugs.openjdk.java.net/browse/JDK-6567433

 

Calling updateUI() on JList, JComboBox and JTableHeader can create 
StackOverflowErrors.
For example -

JList.updateUI() invokes updateUI() on its Cellrenderer via 
SwingUtilities.updateComponentTreeUI().
If the cellrenderer is a parent of this JList the method recurses endless 
causing StackOverflowError.

 

 

Fix :

 Added a recursion guard to JComboBox, JList and JTableHeader classes.

 With this fix, UpdateUI() method in these classes does not result in 
recursion.

 

Webrev :

http://cr.openjdk.java.net/~aghaisas/6567433/webrev.00/

 

Request you to review.

 

Regards,

Ajit