This is a very good use case for understanding static initialization.
My understanding is that if you have a static initializer such as:
private static WebServiceEngineImpl INSTANCE = new
WebServiceEngineImpl();
then you can use a non-synchronized accessor,
public static /*synchronized*/ WebServiceEngineImpl getInstance() {
return INSTANCE;
}
because initialization of a static is a special case of thread-safe
access.
But this will completely initialize the WebServiceEngineImpl instance
before completing the initialization of the caller, including the
static initialization of the WebServiceEngineImpl class itself.
If you want to lazily initialize a component you must use the
synchronization primitive either explicitly with the synchronized
{ ... } or the synchronized keyword on the method, as
public static synchronized WebServiceEngineFactory getInstance() {
if (_factory==null) {
_factory = new WebServiceEngineFactory();
}
return _factory;
}
This will synchronize every thread's access to the
WebServiceEngineFactory as a tradeoff for the ability to lazily
initialize it. The tradeoff means that you should consider whether a
component will always be needed (use static initializer) or might not
be needed (use lazy initialization).
Craig
P.S. I'm glad to see that no one suggested using the broken double
checked lock as a solution!
On Dec 7, 2006, at 2:28 PM, Lloyd L Chambers wrote:
> Yes, thanks, my oversight.
>
> On Dec 7, 2006, at 1:49 PM, Dhiru Pandey wrote:
>
>> Hello Lloyd,
>>
>> Looks fine but really no need for synchronized in :
>>
>> public static synchronized WebServiceEngineImpl getInstance() {
>> return INSTANCE;
>> }
>>
>> Also, you may want to do the same thing for:
>>
>> public static synchronized WebServiceEngineFactory getInstance
>> () {
>> if (_factory==null) {
>> _factory = new WebServiceEngineFactory();
>> }
>> return _factory;
>> }
>>
>>
>> Lloyd L Chambers wrote:
>>> Dhiru,
>>>
>>> Your suggestion is a good one. Assuming the class is never
>>> loaded before it's needed, using a static initializer is not
>>> wasteful.
>>>
>>> The original code was quite tricky (two intertwined classes) and
>>> your suggestion wasn't feasible. I've simplified it down to
>>> this now:
>>>
>>> public class WebServiceEngineImpl implements WebServiceEngine {
>>>
>>> /** Creates a new instance of TracingSystemHandlerFactory */
>>> protected WebServiceEngineImpl() {
>>> addAuthListener( new LogAuthenticationListener() );
>>> }
>>>
>>> private static final WebServiceEngineImpl INSTANCE = new
>>> WebServiceEngineImpl();
>>>
>>> public static synchronized WebServiceEngineImpl getInstance() {
>>> return INSTANCE;
>>> }
>>> .......
>>> }
>>>
>>>
>>> public class WebServiceEngineFactory {
>>>
>>> private static WebServiceEngineFactory _factory = null;
>>>
>>> private final WebServiceEngine engine;
>>>
>>> /** Creates a new instance of WebServiceEngineFactory */
>>> protected WebServiceEngineFactory() {
>>> // build-order problem--have to use reflection
>>> try {
>>> final ClassLoader classLoader = Thread.currentThread
>>> ().getContextClassLoader();
>>> final Class engineClass =
>>> classLoader.loadClass
>>> ( "com.sun.enterprise.webservice.monitoring.WebServiceEngineImpl" );
>>> final java.lang.reflect.Method m =
>>> engineClass.getDeclaredMethod( "getInstance", (Class[])null);
>>> engine = (WebServiceEngine)m.invoke( null, (Object[])
>>> null );
>>> }
>>> catch( final Exception e ) {
>>> throw new Error( e );
>>> }
>>> }
>>>
>>> /**
>>> * @return the singleton Factory implementation
>>> */
>>> public static synchronized WebServiceEngineFactory getInstance
>>> () {
>>> if (_factory==null) {
>>> _factory = new WebServiceEngineFactory();
>>> }
>>> return _factory;
>>> }
>>>
>>> /**
>>> * @return this appserver WebServiceEngine instance
>>> */
>>> public WebServiceEngine getEngine() {
>>> return engine;
>>> }
>>> }
>>>
>>>
>>> Lloyd
>>>
>>> On Dec 7, 2006, at 11:40 AM, Dhiru Pandey wrote:
>>>
>>>> Why not do this:
>>>>
>>>> private static WebServiceEngineImpl INSTANCE = new
>>>> WebServiceEngineImpl();
>>>>
>>>> public static WebServiceEngineImpl getInstance() {
>>>> return INSTANCE;
>>>> }
>>>>
>>>> instead of:
>>>>
>>>> + private static WebServiceEngineImpl INSTANCE = null;
>>>> +
>>>> + public static synchronized WebServiceEngineImpl getInstance
>>>> () {
>>>> + if ( INSTANCE == null ) {
>>>> + INSTANCE = new WebServiceEngineImpl();
>>>> }
>>>> - return (WebServiceEngineImpl) factory.getEngine();
>>>> + return INSTANCE;
>>>> }
>>>>
>>>> You would save on the synchronization cost if getInstance() call
>>>> is called by multiple threads
>>>>
>>>> -Dhiru
>>>>
>>>> Lloyd L Chambers wrote:
>>>>> Jerome,
>>>>>
>>>>> Thanks...
>>>>>
>>>>> But it seems I was hasty--it is more complicated due to an
>>>>> interaction between the factory and the impl (see my other
>>>>> email). And there is a build-order problem too.
>>>>>
>>>>> Here are new diffs which fix the problem I'm seeing with PE.
>>>>> Is there a different approach going on in EE? (it doesn't
>>>>> appear so as there is no -D business going on).
>>>>>
>>>>> MB2:/gf/build/glassfish lloyd$ cvs diff -u appserv-core/src/
>>>>> java/com/sun/enterprise/webservice/monitoring/
>>>>> WebServiceEngineImpl.java appserv-commons/src/java/com/sun/
>>>>> enterprise/webservice/monitoring/WebServiceEngineFactory.java
>>>>> Index: appserv-core/src/java/com/sun/enterprise/webservice/
>>>>> monitoring/WebServiceEngineImpl.java
>>>>> ==================================================================
>>>>> =
>>>>> RCS file: /cvs/glassfish/appserv-core/src/java/com/sun/
>>>>> enterprise/webservice/monitoring/WebServiceEngineImpl.java,v
>>>>> retrieving revision 1.14
>>>>> diff -u -r1.14 WebServiceEngineImpl.java
>>>>> --- appserv-core/src/java/com/sun/enterprise/webservice/
>>>>> monitoring/WebServiceEngineImpl.java 18 Nov 2006 02:48:20
>>>>> -0000 1.14
>>>>> +++ appserv-core/src/java/com/sun/enterprise/webservice/
>>>>> monitoring/WebServiceEngineImpl.java 7 Dec 2006 18:12:17 -0000
>>>>> @@ -58,6 +58,8 @@
>>>>> import com.sun.logging.LogDomains;
>>>>> import com.sun.enterprise.deployment.WebServiceEndpoint;
>>>>> +import
>>>>> com.sun.enterprise.webservice.monitoring.LogAuthenticationListener
>>>>> ;
>>>>> +
>>>>> /**
>>>>> * This class acts as a factory to create TracingSystemHandler
>>>>> * instances. It also provides an API to register listeners
>>>>> @@ -74,27 +76,22 @@
>>>>> new ArrayList<AuthenticationListener>();
>>>>> protected GlobalMessageListener globalMessageListener = null;
>>>>>
>>>>> -
>>>>> - static WebServiceEngineFactory factory =
>>>>> WebServiceEngineFactory.getInstance();
>>>>> static ThreadLocal servletThreadLocal = new ThreadLocal();
>>>>> public static Logger sLogger = LogDomains.getLogger
>>>>> (LogDomains.CORE_LOGGER);
>>>>>
>>>>>
>>>>> /** Creates a new instance of TracingSystemHandlerFactory */
>>>>> protected WebServiceEngineImpl() {
>>>>> + addAuthListener( new LogAuthenticationListener() );
>>>>> }
>>>>>
>>>>> - public static WebServiceEngineImpl getInstance() {
>>>>> -
>>>>> - if (factory.getEngine()==null) {
>>>>> - synchronized(factory) {
>>>>> - if (factory.getEngine()==null) {
>>>>> - factory.setEngine(new WebServiceEngineImpl
>>>>> ());
>>>>> - factory.getEngine().addAuthListener(new
>>>>> LogAuthenticationListener());
>>>>> - }
>>>>> - }
>>>>> + private static WebServiceEngineImpl INSTANCE = null;
>>>>> +
>>>>> + public static synchronized WebServiceEngineImpl getInstance
>>>>> () {
>>>>> + if ( INSTANCE == null ) {
>>>>> + INSTANCE = new WebServiceEngineImpl();
>>>>> }
>>>>> - return (WebServiceEngineImpl) factory.getEngine();
>>>>> + return INSTANCE;
>>>>> }
>>>>>
>>>>> public EndpointImpl createHandler(WebServiceEndpoint
>>>>> endpointDesc) {
>>>>> Index: appserv-commons/src/java/com/sun/enterprise/webservice/
>>>>> monitoring/WebServiceEngineFactory.java
>>>>> ==================================================================
>>>>> =
>>>>> RCS file: /cvs/glassfish/appserv-commons/src/java/com/sun/
>>>>> enterprise/webservice/monitoring/WebServiceEngineFactory.java,v
>>>>> retrieving revision 1.3
>>>>> diff -u -r1.3 WebServiceEngineFactory.java
>>>>> --- appserv-commons/src/java/com/sun/enterprise/webservice/
>>>>> monitoring/WebServiceEngineFactory.java 25 Dec 2005
>>>>> 04:12:42 -0000 1.3
>>>>> +++ appserv-commons/src/java/com/sun/enterprise/webservice/
>>>>> monitoring/WebServiceEngineFactory.java 7 Dec 2006
>>>>> 18:12:17 -0000
>>>>> @@ -20,10 +20,10 @@
>>>>> *
>>>>> * Copyright 2006 Sun Microsystems, Inc. All rights reserved.
>>>>> */
>>>>> -
>>>>> -
>>>>> package com.sun.enterprise.webservice.monitoring;
>>>>> +import java.lang.reflect.Method;
>>>>> +
>>>>> /**
>>>>> * Factory for the WebServiceEngine
>>>>> *
>>>>> @@ -31,26 +31,31 @@
>>>>> */
>>>>> public class WebServiceEngineFactory {
>>>>>
>>>>> - private static WebServiceEngineFactory factory = null;
>>>>> + private static WebServiceEngineFactory _factory = null;
>>>>>
>>>>> private WebServiceEngine engine = null;
>>>>>
>>>>> /** Creates a new instance of WebServiceEngineFactory */
>>>>> protected WebServiceEngineFactory() {
>>>>> + // build-order problem
>>>>> + try {
>>>>> + final Class engineClass = Class.forName
>>>>> ( "com.sun.enterprise.webservice.monitoring.WebServiceEngineImpl"
>>>>> );
>>>>> + final java.lang.reflect.Method m =
>>>>> engineClass.getDeclaredMethod( "getInstance", (Class[])null);
>>>>> + engine = (WebServiceEngine)m.invoke( null, (Object
>>>>> [])null );
>>>>> + }
>>>>> + catch( final Exception e ) {
>>>>> + throw new Error( e );
>>>>> + }
>>>>> }
>>>>>
>>>>> /**
>>>>> * @return the singleton Factory implementation
>>>>> */
>>>>> - public static WebServiceEngineFactory getInstance() {
>>>>> - if (factory==null) {
>>>>> - synchronized(WebServiceEngineFactory.class) {
>>>>> - if (factory==null) {
>>>>> - factory = new WebServiceEngineFactory();
>>>>> - }
>>>>> - }
>>>>> - }
>>>>> - return factory;
>>>>> + public static synchronized WebServiceEngineFactory
>>>>> getInstance() {
>>>>> + if (_factory==null) {
>>>>> + _factory = new WebServiceEngineFactory();
>>>>> + }
>>>>> + return _factory;
>>>>> }
>>>>>
>>>>> /**
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Dec 7, 2006, at 9:25 AM, Jerome Dochez wrote:
>>>>>
>>>>>> looks good
>>>>>> On Dec 7, 2006, at 8:56 AM, Lloyd L Chambers wrote:
>>>>>>
>>>>>>> See diffs in bug:
>>>>>>>
>>>>>>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1685
>>>>>>>
>>>>>>> Lloyd Chambers
>>>>>>>
>>>>>>> ----------------------------------------------------------------
>>>>>>> -----
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>>>>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>>>>>
>>>>>>
>>>>>> -----------------------------------------------------------------
>>>>>> ----
>>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>>>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>>>>
>>>>>
>>>>> ------------------------------------------------------------------
>>>>> ---
>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>>>
>>>
>>> --------------------------------------------------------------------
>>> -
>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>
>
Craig Russell
Architect, Sun Java Enterprise System
http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell_at_sun.com
P.S. A good JDO? O, Gasp!
- application/pkcs7-signature attachment: smime.p7s