Code review request: 7063647, jsse/runtime, To use synchronized map in key manager

[Xuelei Fan]

On 8/9/2011 8:03 AM, Brad Wetmore wrote:
> Why do you need the "syncronchized (credentialsMap)" at line 344? Aren't
> all writes done during the constructor init?
>
The credentialsMap.entrySet() returns the reference of the
backed/internal set, the iteration of the set is not thread safe. The
Collections.synchronizedMap specification requires:
------------------
It is imperative that the user manually synchronize on the returned map
when iterating over any of its collection views:

  Map m = Collections.synchronizedMap(new HashMap());
      ...
  Set s = m.keySet();  // Needn't be in synchronized block
      ...
  synchronized (m) {  // Synchronizing on m, not s!
      Iterator i = s.iterator(); // Must be in synchronized block
      while (i.hasNext())
          foo(i.next());
  }


Failure to follow this advice may result in non-deterministic behavior.
------------------

Thanks for the code review.

Xuelei

> Otherwise, looks ok.
> 
> Brad
> 
> 
> On 8/7/2011 8:43 PM, Xuelei Fan wrote:
>> webrev: http://cr.openjdk.java.net/~xuelei/7063647/webrev.00/
>>
>> SunX509KeyManagerImpl should be multiple thread safe, need to
>> synchronize cached map:
>>      private Map<String,X509Credentials>  credentialsMap;
>>      private Map<String,String[]>  serverAliasCache;
>>
>> Thanks,
>> Xuelei