<AWT Dev> <Swing Dev> Review request #4: 6852592 (invalidate() must be smarter) - approved

[Anthony Petrov]

On 10/8/2009 12:34 PM Artem Ananiev wrote:
>>>>> Artem, would you agree on placing all calls to the isValid() under 
>>>>> the TreeLock?
>>>>
>>>> Yes, that would be fine. Have we already introduced a warning about all 
>>>
>>> OK, I'll modify the fix for 6887249 accordingly.
>>
>> Well, I revised the code, and it appears that the 'valid' boolean 
>> field is declared volatile. Which basically means that we should only 
>> acquire a lock when we need an atomic "read-then-update" sort of 
>> operation (like validate() or invalidate() do.) When we need to read 
>> the value of this field only w/o subsequent updating it, we don't 
>> actually need any locking at all. So I tend to think that the fix for 
>> 6887249 should modify the Container.validate() method only. What do 
>> you think?
> 
> If isValid() were a final method that just returns a value of 'isValid' 
> field, then yes, we wouldn't have to provide any external 
> synchronization. However, users might want to override isValid(), so I'd 
> better place all the calls to isValid() under the tree lock.

What about the Component.paramString() method then? Couldn't it produce 
some dead-locks while debugging is in progress?

Also, there's a number of isValid() calls in the Swing code (e.g., 
JViewport, BasicTabbedPaneUI, and possibly some more.) Should these be 
modified as well? Alex, what's your opinion?

--
best regards,
Anthony