dev@glassfish.java.net

Re: CODE REVIEW: WebServiceEngine thread-unsafe initialization bugs #1694, #1695

From: Dhiru Pandey <Dhiru.Pandey_at_Sun.COM>
Date: Thu, 07 Dec 2006 11:40:15 -0800

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
>