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
>>