Hi Lloyd,
Thanks, I recently had the great pleasure to hear one of the experts
on the Java Memory Model, Bill Pugh, talk about the updates to the
Model, and I learned a lot. In particular, I had not realized that
static initialization of a static instance was guaranteed to be
thread-safe. That one point alone was worth the price of attendance.
Craig
On Dec 8, 2006, at 1:01 PM, Lloyd L Chambers wrote:
> Craig,
>
> Well-stated, and I hope the audience takes the time to read it--I
> am seeing tons of thread-unsafe methods, fixed about 15 already.
>
> Lloyd
>
> On Dec 7, 2006, at 5:38 PM, Craig L Russell wrote:
>
>> 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.LogAuthenticationListen
>>>>>>> er;
>>>>>>> +
>>>>>>> /**
>>>>>>> * 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!
>>
>
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