<core-libs-dev> sun.management.Agent: the properties.putAll API may fail with ConcurrentModifcationException on multi-thread scenario

[Deven You]

Hi Dmitry,

I have verified the change set. Thanks for your commit!

On 05/17/2012 03:44 PM, Dmitry Samersoff wrote:
> Deven,
>
> The fix is submitted to openjdk  tl workspace
>
>    http://hg.openjdk.java.net/jdk8/tl/jdk/rev/df33f5f750ec
>
> -Dmitry
>
>
> On 2012-04-26 05:21, Deven You wrote:
>> Hi Dmitry,
>>
>> Thanks for your help. I have created a CR with internal id 2236492 which
>> hasn't be published yet. So please set this internal CR id as duplicate
>> to 716419 as well.
>>
>> This is the original mail for this problem:
>>
>>
>>
>> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>
>> Hi core-libs-devs,
>>
>> I am not sure if sun.management.Agent belongs to jmx-dev mailing list,
>> if so please anyone tell me.
>>
>> This issue is that the sun.management.Agent.loadManagementProperties()
>> will invoke properties.putAll which will throw
>> ConcurrentModifcationException if there are other threads which modify
>> the properties concurrently.
>>
>> I have made a patch[1] which synchronize the sysProps so that putAll can
>> work on multi-thread scenario. The test case is also available in [1].
>>
>> Thanks a lot!
>>
>> [1] http://cr.openjdk.java.net/~littlee/OJDK-256/webrev.00
>> <http://cr.openjdk.java.net/%7Elittlee/OJDK-256/webrev.00>
>>
>> ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>
>>
>> Thanks a lot!
>>
>>
>>
>> On 04/25/2012 08:32 PM, Dmitry Samersoff wrote:
>>> Deven,
>>>
>>> CR number is  7164191 .
>>>
>>> Could you re-send me your original e-mail with problem description and
>>> webrev link.
>>>
>>>    I'll put it to CR comment field.
>>>
>>>
>>> -Dmitry
>>>
>>>
>>>
>>> On 2012-04-24 16:15, Dmitry Samersoff wrote:
>>>> Deven,
>>>>
>>>> After close look and off-line discussion with David Holmes,
>>>> the changes looks good for me.
>>>>
>>>> I'll take care of the rest.
>>>>
>>>> We have one more place in Agent.java executing exactly the same code
>>>> so I'll change both of them on your behalf.
>>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2012-04-23 11:43, Deven You wrote:
>>>>> Thanks David,
>>>>>
>>>>> So is it ok for you to contribute this patch?
>>>>>
>>>>> On 04/23/2012 02:36 PM, David Holmes wrote:
>>>>>> Except of course that Properties is a Hashtable and synchronizes on
>>>>>> 'this' for all public methods. So locking the properties object in the
>>>>>> client code will guarantee exclusive access to it.
>>>>>>
>>>>>> Sorry about that.
>>>>>>
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>> On 23/04/2012 4:30 PM, David Holmes wrote:
>>>>>>> Deven,
>>>>>>>
>>>>>>> On 23/04/2012 3:54 PM, Deven You wrote:
>>>>>>>> On 04/18/2012 02:20 PM, Deven You wrote:
>>>>>>>>> On 04/18/2012 01:34 PM, Mandy Chung wrote:
>>>>>>>>>> I think this could still run into CME. System Properties is not a
>>>>>>>>>> synchronized map and the setter methods (System.setProperty or
>>>>>>>>>> Properties.put method) doesn't synchronize on the Properties
>>>>>>>>>> object.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The setter methods I'm referring to are System.setProperty and
>>>>>>>>>> System.getProperties().put().
>>>>>>>>>>
>>>>>>>>> I have gone through the Agent.java, I think other set/put methods
>>>>>>>>> related to properties are protected properly.
>>>>>>>>>
>>>>>>>>> public static void agentmain using parseString(args) which return a
>>>>>>>>> properties which is a local var and is not possible to cause
>>>>>>>>> concurrent problem when call config_props.putAll(arg_props).
>>>>>>>>>
>>>>>>>>> private static synchronized void startLocalManagementAgent() is
>>>>>>>>> synchronized already.
>>>>>>>>>
>>>>>>>>> private static synchronized void startRemoteManagementAgent(String
>>>>>>>>> args) is synchronized also.
>>>>>>>>>
>>>>>>>>> Could you point where the CME may ocurr?
>>>>>>>> Is there any suggestion from the mailing list?
>>>>>>> The problem is that System.getProperties() returns a globally
>>>>>>> accessible
>>>>>>> set of properties. So even if you prevent the Agent code from
>>>>>>> modifying
>>>>>>> those properties concurrently with other use in the Agent, you
>>>>>>> have no
>>>>>>> such guard for any other piece of code in the system which might also
>>>>>>> modify the properties. So the race condition you were trying to fix
>>>>>>> still exists. I don't see any way to fix this. No matter what you do
>>>>>>> another thread can modify the system properties while you are
>>>>>>> iterating
>>>>>>> them. Instead you need to anticipate the CME and try to recover
>>>>>>> from it
>>>>>>> (also non-trivial).
>>>>>>>
>>>>>>> Cheers,
>>>>>>> David Holmes
>>
>


-- 
Best Regards,

Deven