jmx-dev Call for review, Bug ID 6514985

[Jean-Francois Denise - Sun Microsystems]

Rethinking at this fix, the access to the ServiceLoader iterator inside 
a privileged block is useless (and used to be useless).
ServiceLoader javadoc is quite vague on this but ServiceLoader code is 
clear.

After discussing with Eamonn, I confidently removed the grant block.

You can check the updated webrev on : 
http://amos.france.sun.com/jmgt/user/jfdenise/workspaces/jdk7_jmx_master.ws/webrev/index.html 

(Not reviewed by community, not adding diff).

JF.

Eamonn McManus wrote:
> This fix looks good to me.
>
> Éamonn McManus   JMX Spec Lead   http://weblogs.java.net/blog/emcmanus/
>
>
>
> Jean-Francois Denise - Sun Microsystems wrote:
>> Proposal to fix 6514985 : JMX should use java.util.ServiceLoader to 
>> lookup service providers
>>
>> Swan access webrev
>> ==================
>>
>> http://amos.france.sun.com/jmgt/user/jfdenise/workspaces/jdk7_jmx_master.ws/webrev/index.html 
>>
>>
>> Diff
>> ====
>>
>> diff javax/management/remote/JMXConnectorFactory.java
>> /jmgt/master/jmx/jdk7_jmx_master.ws/src/share/classes/javax/management/remote//JMXConnectorFactory.java 
>>
>> 34d33
>> < import java.util.ServiceLoader;
>> 40a40
>>> import com.sun.jmx.remote.util.Service;
>> 42d41
>> <
>> 422,429c421,428
>> <     static <T> Iterator<T> getProviderIterator(final ServiceLoader<T>
>> loader) {
>> <         PrivilegedAction<Iterator<T>>
>> <                 action = new PrivilegedAction<Iterator<T>>() {
>> <             public Iterator<T> run() {
>> <                 return loader.iterator();
>> <             }
>> <         };
>> <         return AccessController.doPrivileged(action);
>> ---
>>>     static Iterator getProviderIterator(final Class providerClass,
>>>                                         final ClassLoader loader) {
>>>         PrivilegedAction action = new PrivilegedAction() {
>>>                 public Object run() {
>>>                     return Service.providers(providerClass, loader);
>>>                 }
>>>             };
>>>         return (Iterator) AccessController.doPrivileged(action);
>> 436,441c435,438
>> <
>> <         ServiceLoader<JMXConnectorProvider> serviceLoader =
>> <                 ServiceLoader.load(JMXConnectorProvider.class,
>> <                 loader);
>> <         Iterator<JMXConnectorProvider> providers =
>> <                 getProviderIterator(serviceLoader);
>> ---
>>>
>>>         Iterator providers = 
>> getProviderIterator(JMXConnectorProvider.class,
>>>                                                  loader);
>>>         JMXConnectorProvider provider = null;
>> 444c441,442
>> <         while(providers.hasNext()) {
>> ---
>>>         while (providers.hasNext()) {
>>>             provider = (JMXConnectorProvider) providers.next();
>> 446c444
>> <                 connection = providers.next().newJMXConnector(url, 
>> map);
>> ---
>>>                 connection = provider.newJMXConnector(url, map);
>> diff javax/management/remote/JMXConnectorServerFactory.java
>> /jmgt/master/jmx/jdk7_jmx_master.ws/src/share/classes/javax/management/remote//JMXConnectorServerFactory.java 
>>
>> 38d37
>> < import java.util.ServiceLoader;
>> 214,219c213,215
>> <         ServiceLoader<JMXConnectorServerProvider> serviceLoader =
>> <                 ServiceLoader.load(JMXConnectorServerProvider.class,
>> <                 loader);
>> <         Iterator<JMXConnectorServerProvider> providers =
>> <                 
>> JMXConnectorFactory.getProviderIterator(serviceLoader);
>> <
>> ---
>>>         Iterator providers = JMXConnectorFactory.getProviderIterator(
>>>             JMXConnectorServerProvider.class, loader);
>>>         JMXConnectorServerProvider provider = null;
>> 222a219
>>>             provider = (JMXConnectorServerProvider) providers.next();
>> 224c221
>> <                 connection =
>> providers.next().newJMXConnectorServer(url, map, mbs);
>> ---
>>>                 connection = provider.newJMXConnectorServer(url, map, 
>> mbs);
>>
>> Only in
>> /jmgt/master/jmx/jdk7_jmx_master.ws/src/share/classes/com/sun/jmx/remote/util/: 
>>
>> Service.java
>> Only in deleted_files//src/share: classes
>>
>>
>


-- 
Jean-Francois Denise  JMX team
Interested in netbeans ==> http://blogs.sun.com/roller/page/jmxnetbeans