Daniel,

Given that helper.loadClass uses the context class loader, Should you also simply use ServiceLoader<InitialContextFactory> loader = ServiceLoader.load(InitialContextFactory.class); at lines 680-681 ?

It needs to be the system class loader to allow for JNDI providers that might be on the class path or module path.

Also it might be good to log an RFE against the ServiceLoader, so that you could look for a service implementation of a specific concrete class without having to instantiate all the other service implementations encountered along the way. Streams should provide a nice infrastructure for such an API. It would certainly be more robust than looping over ServiceLoader.iterator().next() - which is unfortunately the only option available to you at the moment.

IMO, it's not just an inconvenience, but rather a part of ServiceLoader's design. I mean, it's definitely designed to provide, so to say, a "one-to-many" mapping for the classes (providers) that implements some interface (a service) to a client. It merely delivers you implementations. You should than iterate though them and decide which one satisfies your needs. I'm not sure it's a good idea to get services based on their implementation classnames. It's more likely to be a little bit a flaw in design from the JNDI part -- when you have to specify the implementation class's FQN. But given the history of the JNDI (it's started as a project outside the JDK) -- it's totally understandable. (Just as a side-note, have a look at these examples of usage of ServiceLoader: java.net.InetAddress:862 java.time.chrono.AbstractChronology:274 java.time.chrono.AbstractChronology:307 javax.xml.validation.SchemaFactoryFinder:405 javax.xml.xpath.XPathFactoryFinder:403 java.time.zone.ZoneRulesProvider:177)

Also - it would be good to clarify the specification of public static Context getInitialContext(Hashtable<?,?> env) It was not clear to me that you would loop over all the services found by the ServiceLoader until you'd find one whose concrete class matched the name pointed to by Context.INITIAL_CONTEXT_FACTORY.

Don't you think it becomes than 'overspecified'? Why should we want to tie ourselves?

This seems a bit fragile to me - unless it's guaranteed that the various InitialContextFactory have no static initializer that might throw exceptions (such as SecurityException) - and that their default constructor does nothing (so that instantiating e.g. com.sun.jndi.cosnaming.CNCtxFactory when you're actually looking for com.sun.jndi.ldap.LdapCtxFactory has no side effect - which fortunately seems to be the case).

Well, no one can guarantee us this. Even a constructor could do all the things you've mentioned :) It's just the nature of a factory. It should better be stateless and without any side effects. -Pavel

On 9/16/14 4:14 PM, Pavel Rappo wrote:

Daniel,

...

Given that helper.loadClass uses the context class loader, Should you also simply use ServiceLoader<InitialContextFactory> loader = ServiceLoader.load(InitialContextFactory.class); at lines 680-681 ?

It needs to be the system class loader to allow for JNDI providers that might be on the class path or module path.

So is it expected that modules (e.g. java.corba) will register their own service provider for the InitialContextFactory (I mean - using META-INF/services/)?

...

Also it might be good to log an RFE against the ServiceLoader, so that you could look for a service implementation of a specific concrete class without having to instantiate all the other service implementations encountered along the way. Streams should provide a nice infrastructure for such an API. It would certainly be more robust than looping over ServiceLoader.iterator().next() - which is unfortunately the only option available to you at the moment.

IMO, it's not just an inconvenience, but rather a part of ServiceLoader's design. I mean, it's definitely designed to provide, so to say, a "one-to-many" mapping for the classes (providers) that implements some interface (a service) to a client. It merely delivers you implementations. You should than iterate though them and decide which one satisfies your needs. I'm not sure it's a good idea to get services based on their implementation classnames. It's more likely to be a little bit a flaw in design from the JNDI part -- when you have to specify the implementation class's FQN. But given the history of the JNDI (it's started as a project outside the JDK) -- it's totally understandable.

Right. It's not usual to use the service loader to look for a specific concrete implementation class name. So on the one side what you're doing may be better than modifying ServiceLoader to support an unusual usage. I can buy this.

(Just as a side-note, have a look at these examples of usage of ServiceLoader:

[snip]

javax.xml.validation.SchemaFactoryFinder:405 javax.xml.xpath.XPathFactoryFinder:403 [snip]

I know these two ;-) I updated JAXP to use ServiceLoader :-) The difference however is that in this two case we're looking for a service implementing a specific feature, we're not looking for a service whose concrete class matches a specific class name. In the case of InitialContextFactory - as a client of the API, I would be a bit surprised if - asking for a LDAP InitialContext, I received a CORBA exception. This would look strange to me. But maybe there's nothing you can do given how JNDI is currently working. A possibilty might be to use InitialContextFactoryBuilder as the service interface instead, and loop over the implementations until you find one that does not throw NamingException - but then it would be difficult to make the distinction between 'This builder does not support the Context.INITIAL_CONTEXT_FACTORY that you ask for' and 'The InitialContextFactory could not be instantiated for various reasons'... So I'm not sure it would be better.

java.time.zone.ZoneRulesProvider:177)

...

Also - it would be good to clarify the specification of public static Context getInitialContext(Hashtable<?,?> env) It was not clear to me that you would loop over all the services found by the ServiceLoader until you'd find one whose concrete class matched the name pointed to by Context.INITIAL_CONTEXT_FACTORY.

Don't you think it becomes than 'overspecified'? Why should we want to tie ourselves?

Maybe it's because I haven't used JNDI very often, but when I read the spec I didn't realize that the value of Context.INITIAL_CONTEXT_FACTORY would have any effect if ServiceLoader was used - because it's not usual to use ServiceLoader in this way (even though the usage of ServiceLoader is nested below 'the class specified in the Context.INITIAL_CONTEXT_FACTORY</tt> environment property is used'). I'm not sure it would be overspecifying to say that the ServiceLoader is used to locate and load an implementation of the service whose concrete class matches the named class - since that's what it's doing anyway - but I understand your concern.

...

This seems a bit fragile to me - unless it's guaranteed that the various InitialContextFactory have no static initializer that might throw exceptions (such as SecurityException) - and that their default constructor does nothing (so that instantiating e.g. com.sun.jndi.cosnaming.CNCtxFactory when you're actually looking for com.sun.jndi.ldap.LdapCtxFactory has no side effect - which fortunately seems to be the case).

Well, no one can guarantee us this. Even a constructor could do all the things you've mentioned :) It's just the nature of a factory. It should better be stateless and without any side effects.

So a broken InitialContextFactory for technology XXX can prevent getting the InitialContext for technology YYY even if both have nothing in common, and even if XXX is never actually used in the application. On the other hand I don't see that you have any other choice but propagate the exception if one is emitted - as you can't know whether it has been emitted by the class you're trying to load :-( -- daniel

-Pavel