dev@javaserverfaces.java.net

Re: Call for code review for BugDB 19859648

From: manfred riem <manfred.riem_at_oracle.com>
Date: Mon, 27 Oct 2014 10:22:33 -0500

Hi Zhijun,

Provided that it runs all the tests pass on both Weblogic and GF you
have r=mriem.

Thanks!
Manfred

On 10/23/14, 11:31 PM, zhijun Ren wrote:
> Hi Manfred and Ed,
>
> Changed bundle text attached here, please help to review.
>
> I plan to commit this fix to 2.2.9, is that right for this wls2.2.1 bug?
>
> BR,
> Zhijun
>
> On 10/24/14, 10:37, zhijun Ren wrote:
>> Thanks Manfred, I will send out the review quest today.
>>
>> On 10/23/14, 22:53, manfred riem wrote:
>>> Hi Zhijun,
>>>
>>> OK go ahead and do that change and please run all the tests
>>>
>>> Thanks!
>>> Manfred
>>>
>>> On 10/22/14, 10:21 PM, zhijun Ren wrote:
>>>> Hi Manfred and Ed,
>>>>
>>>> The WeakHashMap acts as a cache for the MetaDataTarget here and it
>>>> is static so there is one instance under one classloader. The
>>>> concurrency accessing wasn't considered when the class was designed.
>>>>
>>>> Race condition comes in the MT environment, but it impacts not only
>>>> this class but also the whole JSF framework, so we need to consider
>>>> the integration of MT and JSF as a whole picture in terms of
>>>> concurrency.
>>>>
>>>> So I think, changing the WeakHashMap by wrapping with synchronized
>>>> makes sense for this bug in the current view, do you agree?
>>>>
>>>> BR,
>>>> Zhijun
>>>>
>>>> On 10/22/14, 22:00, manfred riem wrote:
>>>>> Hi Zhijun,
>>>>>
>>>>> We need to understand what it is that this particular map is
>>>>> having the static WeakHashMap and if it might need to be replaced
>>>>> by a non-static variant of this. What is this map used for?
>>>>>
>>>>> Thanks!
>>>>> Manfred
>>>>>
>>>>> On 10/21/14, 9:57 PM, zhijun Ren wrote:
>>>>>> Hi ED and Manfred,
>>>>>>
>>>>>> This bug's root cause is that in class
>>>>>> com.sun.faces.facelets.tag.MetaRulesetImpl, its class
>>>>>> variable(static), an instance of WeakHashmap, is accessed by
>>>>>> multiple threads in the case of MT and it is not synchronized.
>>>>>>
>>>>>> My question is:
>>>>>>
>>>>>> 1. In MT case, should it be accessed by multiple threads?
>>>>>>
>>>>>> 2. If yes, the variable should be a synchronized map; otherwise,
>>>>>> the class should be used in different naming spaces(under
>>>>>> different classloader), right?
>>>>>>
>>>>>> BR,
>>>>>> Zhijun