You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ja...@apache.org on 2014/09/03 11:10:04 UTC

svn commit: r1622193 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java

Author: jacopoc
Date: Wed Sep  3 09:10:04 2014
New Revision: 1622193

URL: http://svn.apache.org/r1622193
Log:
Greatly simplified the UtilJ2eeCompat class by removing a series of variables and methods no more in use.
 The class was not thread safe and this didn't cause particular issues probably just because all the threads using it were passing ServletContext with the same server info value (because they were associated to the same servlet container); it was however inefficient.
 Now it is thread safe using the DCL pattern which I usually tend to avoid but is useful here.
 However this class should probably go away but before taking a decision I will start a discussion in the dev list.

Modified:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java?rev=1622193&r1=1622192&r2=1622193&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java Wed Sep  3 09:10:04 2014
@@ -28,74 +28,60 @@ public class UtilJ2eeCompat {
 
     public static final String module = UtilJ2eeCompat.class.getName();
 
-    public static final String TOMCAT = "apache tomcat";
-    public static final String ORION = "orion";
-    public static final String RESIN = "resin";
-    public static final String REX_IP = "tradecity";
-    public static final String OC4J = "oracle";
-    public static final String JRUN = "jrun";
-    public static final String JETTY = "jetty";
-    public static final String WEBSPHERE = "websphere";
-
-    protected static Boolean doFlushOnRenderValue = null;
-    protected static Boolean useOutputStreamNotWriterValue = null;
-    protected static Boolean useNestedJspException = null;
-
-    public static boolean doFlushOnRender(ServletContext context) {
-        initCompatibilityOptions(context);
-        return doFlushOnRenderValue.booleanValue();
-    }
-
-    public static boolean useOutputStreamNotWriter(ServletContext context) {
-        initCompatibilityOptions(context);
-        return useOutputStreamNotWriterValue.booleanValue();
-    }
+    private static final String TOMCAT = "apache tomcat";
+    private static final String ORION = "orion";
+    private static final String REX_IP = "tradecity";
+    private static final String JRUN = "jrun";
+    private static final String JETTY = "jetty";
+    private static final String WEBSPHERE = "websphere";
+
+    private volatile static UtilJ2eeCompat instance;
+
+    private final boolean useOutputStreamNotWriter;
+
+    private UtilJ2eeCompat(ServletContext context) {
+        boolean usestream = true;
+        // if context is null use an empty string here which will cause the defaults to be used
+        String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
+
+        Debug.logInfo("serverInfo: " + serverInfo, module);
+
+        if (serverInfo.indexOf(TOMCAT) >= 0) {
+            Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
+            usestream = false;
+        } else if (serverInfo.indexOf(REX_IP) >= 0) {
+            Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
+            usestream = false;
+        } else if (serverInfo.indexOf(JRUN) >= 0) {
+            Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
+            usestream = false;
+        } else if (serverInfo.indexOf(JETTY) >= 0) {
+            Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
+            usestream = false;
+        } else if (serverInfo.indexOf(ORION) >= 0) {
+            Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
+            usestream = false;
+        } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
+            Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
+            usestream = false;
+        }
 
-    public static boolean useNestedJspException(ServletContext context) {
-        initCompatibilityOptions(context);
-        return useNestedJspException.booleanValue();
+        useOutputStreamNotWriter = usestream;
     }
 
-    protected static void initCompatibilityOptions(ServletContext context) {
-        // this check to see if we should flush is done because on most servers this
-        // will just slow things down and not solve any problems, but on Tomcat, Orion, etc it is necessary
-        if (useOutputStreamNotWriterValue == null || doFlushOnRenderValue == null) {
-            boolean doflush = true;
-            boolean usestream = true;
-            boolean nestjspexception = true;
-            // if context is null use an empty string here which will cause the defaults to be used
-            String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
-
-            Debug.logInfo("serverInfo: " + serverInfo, module);
-
-            if (serverInfo.indexOf(RESIN) >= 0) {
-                Debug.logInfo("Resin detected, disabling the flush on the region render from PageContext for better performance", module);
-                doflush = false;
-            } else if (serverInfo.indexOf(REX_IP) >= 0) {
-                Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
-                usestream = false;
-            } else if (serverInfo.indexOf(TOMCAT) >= 0) {
-                Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
-                usestream = false;
-            } else if (serverInfo.indexOf(JRUN) >= 0) {
-                Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
-                usestream = false;
-            } else if (serverInfo.indexOf(JETTY) >= 0) {
-                Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
-                usestream = false;
-            } else if (serverInfo.indexOf(ORION) >= 0) {
-                Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
-                usestream = false;
-                Debug.logInfo("Orion detected, using non-nested JspException", module);
-                nestjspexception = false;
-            } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
-                Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
-                usestream = false;
+    private static UtilJ2eeCompat getInstance(ServletContext context) {
+        if (instance == null) {
+            synchronized (UtilJ2eeCompat.class) {
+                if (instance == null) {
+                    instance = new UtilJ2eeCompat(context);
+                }
             }
-
-            doFlushOnRenderValue = Boolean.valueOf(doflush);
-            useOutputStreamNotWriterValue = Boolean.valueOf(usestream);
-            useNestedJspException = Boolean.valueOf(nestjspexception);
         }
+        return instance;
     }
+
+    public static boolean useOutputStreamNotWriter(ServletContext context) {
+        return getInstance(context).useOutputStreamNotWriter;
+    }
+
 }



Re: svn commit: r1622193 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
That's true also, I must say I did not look at the DCL implementation in UtilJ2eeCompat

Jacques

Le 03/09/2014 17:57, Jacopo Cappellato a écrit :
> This was true before Java 5 but it works since Java 5 IF you use a volatile field to hold the reference.
>
> Jacopo
>
> On Sep 3, 2014, at 3:38 PM, Adrian Crum <ad...@sandglass-software.com> wrote:
>
>> The DCL design pattern will fail 90% of the time. I was able to prove this a while back when I removed the DCL pattern from the transaction factory and connection factory.
>>
>> I recommend using an atomic reference.
>>
>> Adrian Crum
>> Sandglass Software
>> www.sandglass-software.com
>>
>> On 9/3/2014 10:10 AM, jacopoc@apache.org wrote:
>>> Author: jacopoc
>>> Date: Wed Sep  3 09:10:04 2014
>>> New Revision: 1622193
>>>
>>> URL: http://svn.apache.org/r1622193
>>> Log:
>>> Greatly simplified the UtilJ2eeCompat class by removing a series of variables and methods no more in use.
>>> The class was not thread safe and this didn't cause particular issues probably just because all the threads using it were passing ServletContext with the same server info value (because they were associated to the same servlet container); it was however inefficient.
>>> Now it is thread safe using the DCL pattern which I usually tend to avoid but is useful here.
>>> However this class should probably go away but before taking a decision I will start a discussion in the dev list.
>>>
>>> Modified:
>>>     ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
>>>
>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java?rev=1622193&r1=1622192&r2=1622193&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java (original)
>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java Wed Sep  3 09:10:04 2014
>>> @@ -28,74 +28,60 @@ public class UtilJ2eeCompat {
>>>
>>>      public static final String module = UtilJ2eeCompat.class.getName();
>>>
>>> -    public static final String TOMCAT = "apache tomcat";
>>> -    public static final String ORION = "orion";
>>> -    public static final String RESIN = "resin";
>>> -    public static final String REX_IP = "tradecity";
>>> -    public static final String OC4J = "oracle";
>>> -    public static final String JRUN = "jrun";
>>> -    public static final String JETTY = "jetty";
>>> -    public static final String WEBSPHERE = "websphere";
>>> -
>>> -    protected static Boolean doFlushOnRenderValue = null;
>>> -    protected static Boolean useOutputStreamNotWriterValue = null;
>>> -    protected static Boolean useNestedJspException = null;
>>> -
>>> -    public static boolean doFlushOnRender(ServletContext context) {
>>> -        initCompatibilityOptions(context);
>>> -        return doFlushOnRenderValue.booleanValue();
>>> -    }
>>> -
>>> -    public static boolean useOutputStreamNotWriter(ServletContext context) {
>>> -        initCompatibilityOptions(context);
>>> -        return useOutputStreamNotWriterValue.booleanValue();
>>> -    }
>>> +    private static final String TOMCAT = "apache tomcat";
>>> +    private static final String ORION = "orion";
>>> +    private static final String REX_IP = "tradecity";
>>> +    private static final String JRUN = "jrun";
>>> +    private static final String JETTY = "jetty";
>>> +    private static final String WEBSPHERE = "websphere";
>>> +
>>> +    private volatile static UtilJ2eeCompat instance;
>>> +
>>> +    private final boolean useOutputStreamNotWriter;
>>> +
>>> +    private UtilJ2eeCompat(ServletContext context) {
>>> +        boolean usestream = true;
>>> +        // if context is null use an empty string here which will cause the defaults to be used
>>> +        String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
>>> +
>>> +        Debug.logInfo("serverInfo: " + serverInfo, module);
>>> +
>>> +        if (serverInfo.indexOf(TOMCAT) >= 0) {
>>> +            Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> +            usestream = false;
>>> +        } else if (serverInfo.indexOf(REX_IP) >= 0) {
>>> +            Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> +            usestream = false;
>>> +        } else if (serverInfo.indexOf(JRUN) >= 0) {
>>> +            Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> +            usestream = false;
>>> +        } else if (serverInfo.indexOf(JETTY) >= 0) {
>>> +            Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> +            usestream = false;
>>> +        } else if (serverInfo.indexOf(ORION) >= 0) {
>>> +            Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> +            usestream = false;
>>> +        } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
>>> +            Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> +            usestream = false;
>>> +        }
>>>
>>> -    public static boolean useNestedJspException(ServletContext context) {
>>> -        initCompatibilityOptions(context);
>>> -        return useNestedJspException.booleanValue();
>>> +        useOutputStreamNotWriter = usestream;
>>>      }
>>>
>>> -    protected static void initCompatibilityOptions(ServletContext context) {
>>> -        // this check to see if we should flush is done because on most servers this
>>> -        // will just slow things down and not solve any problems, but on Tomcat, Orion, etc it is necessary
>>> -        if (useOutputStreamNotWriterValue == null || doFlushOnRenderValue == null) {
>>> -            boolean doflush = true;
>>> -            boolean usestream = true;
>>> -            boolean nestjspexception = true;
>>> -            // if context is null use an empty string here which will cause the defaults to be used
>>> -            String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
>>> -
>>> -            Debug.logInfo("serverInfo: " + serverInfo, module);
>>> -
>>> -            if (serverInfo.indexOf(RESIN) >= 0) {
>>> -                Debug.logInfo("Resin detected, disabling the flush on the region render from PageContext for better performance", module);
>>> -                doflush = false;
>>> -            } else if (serverInfo.indexOf(REX_IP) >= 0) {
>>> -                Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> -                usestream = false;
>>> -            } else if (serverInfo.indexOf(TOMCAT) >= 0) {
>>> -                Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> -                usestream = false;
>>> -            } else if (serverInfo.indexOf(JRUN) >= 0) {
>>> -                Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> -                usestream = false;
>>> -            } else if (serverInfo.indexOf(JETTY) >= 0) {
>>> -                Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> -                usestream = false;
>>> -            } else if (serverInfo.indexOf(ORION) >= 0) {
>>> -                Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> -                usestream = false;
>>> -                Debug.logInfo("Orion detected, using non-nested JspException", module);
>>> -                nestjspexception = false;
>>> -            } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
>>> -                Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>>> -                usestream = false;
>>> +    private static UtilJ2eeCompat getInstance(ServletContext context) {
>>> +        if (instance == null) {
>>> +            synchronized (UtilJ2eeCompat.class) {
>>> +                if (instance == null) {
>>> +                    instance = new UtilJ2eeCompat(context);
>>> +                }
>>>              }
>>> -
>>> -            doFlushOnRenderValue = Boolean.valueOf(doflush);
>>> -            useOutputStreamNotWriterValue = Boolean.valueOf(usestream);
>>> -            useNestedJspException = Boolean.valueOf(nestjspexception);
>>>          }
>>> +        return instance;
>>>      }
>>> +
>>> +    public static boolean useOutputStreamNotWriter(ServletContext context) {
>>> +        return getInstance(context).useOutputStreamNotWriter;
>>> +    }
>>> +
>>> }
>>>
>>>
>
>

Re: svn commit: r1622193 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java

Posted by Jacopo Cappellato <ja...@gmail.com>.
This was true before Java 5 but it works since Java 5 IF you use a volatile field to hold the reference.

Jacopo

On Sep 3, 2014, at 3:38 PM, Adrian Crum <ad...@sandglass-software.com> wrote:

> The DCL design pattern will fail 90% of the time. I was able to prove this a while back when I removed the DCL pattern from the transaction factory and connection factory.
> 
> I recommend using an atomic reference.
> 
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
> 
> On 9/3/2014 10:10 AM, jacopoc@apache.org wrote:
>> Author: jacopoc
>> Date: Wed Sep  3 09:10:04 2014
>> New Revision: 1622193
>> 
>> URL: http://svn.apache.org/r1622193
>> Log:
>> Greatly simplified the UtilJ2eeCompat class by removing a series of variables and methods no more in use.
>> The class was not thread safe and this didn't cause particular issues probably just because all the threads using it were passing ServletContext with the same server info value (because they were associated to the same servlet container); it was however inefficient.
>> Now it is thread safe using the DCL pattern which I usually tend to avoid but is useful here.
>> However this class should probably go away but before taking a decision I will start a discussion in the dev list.
>> 
>> Modified:
>>    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
>> 
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java?rev=1622193&r1=1622192&r2=1622193&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java Wed Sep  3 09:10:04 2014
>> @@ -28,74 +28,60 @@ public class UtilJ2eeCompat {
>> 
>>     public static final String module = UtilJ2eeCompat.class.getName();
>> 
>> -    public static final String TOMCAT = "apache tomcat";
>> -    public static final String ORION = "orion";
>> -    public static final String RESIN = "resin";
>> -    public static final String REX_IP = "tradecity";
>> -    public static final String OC4J = "oracle";
>> -    public static final String JRUN = "jrun";
>> -    public static final String JETTY = "jetty";
>> -    public static final String WEBSPHERE = "websphere";
>> -
>> -    protected static Boolean doFlushOnRenderValue = null;
>> -    protected static Boolean useOutputStreamNotWriterValue = null;
>> -    protected static Boolean useNestedJspException = null;
>> -
>> -    public static boolean doFlushOnRender(ServletContext context) {
>> -        initCompatibilityOptions(context);
>> -        return doFlushOnRenderValue.booleanValue();
>> -    }
>> -
>> -    public static boolean useOutputStreamNotWriter(ServletContext context) {
>> -        initCompatibilityOptions(context);
>> -        return useOutputStreamNotWriterValue.booleanValue();
>> -    }
>> +    private static final String TOMCAT = "apache tomcat";
>> +    private static final String ORION = "orion";
>> +    private static final String REX_IP = "tradecity";
>> +    private static final String JRUN = "jrun";
>> +    private static final String JETTY = "jetty";
>> +    private static final String WEBSPHERE = "websphere";
>> +
>> +    private volatile static UtilJ2eeCompat instance;
>> +
>> +    private final boolean useOutputStreamNotWriter;
>> +
>> +    private UtilJ2eeCompat(ServletContext context) {
>> +        boolean usestream = true;
>> +        // if context is null use an empty string here which will cause the defaults to be used
>> +        String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
>> +
>> +        Debug.logInfo("serverInfo: " + serverInfo, module);
>> +
>> +        if (serverInfo.indexOf(TOMCAT) >= 0) {
>> +            Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(REX_IP) >= 0) {
>> +            Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(JRUN) >= 0) {
>> +            Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(JETTY) >= 0) {
>> +            Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(ORION) >= 0) {
>> +            Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
>> +            Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        }
>> 
>> -    public static boolean useNestedJspException(ServletContext context) {
>> -        initCompatibilityOptions(context);
>> -        return useNestedJspException.booleanValue();
>> +        useOutputStreamNotWriter = usestream;
>>     }
>> 
>> -    protected static void initCompatibilityOptions(ServletContext context) {
>> -        // this check to see if we should flush is done because on most servers this
>> -        // will just slow things down and not solve any problems, but on Tomcat, Orion, etc it is necessary
>> -        if (useOutputStreamNotWriterValue == null || doFlushOnRenderValue == null) {
>> -            boolean doflush = true;
>> -            boolean usestream = true;
>> -            boolean nestjspexception = true;
>> -            // if context is null use an empty string here which will cause the defaults to be used
>> -            String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
>> -
>> -            Debug.logInfo("serverInfo: " + serverInfo, module);
>> -
>> -            if (serverInfo.indexOf(RESIN) >= 0) {
>> -                Debug.logInfo("Resin detected, disabling the flush on the region render from PageContext for better performance", module);
>> -                doflush = false;
>> -            } else if (serverInfo.indexOf(REX_IP) >= 0) {
>> -                Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(TOMCAT) >= 0) {
>> -                Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(JRUN) >= 0) {
>> -                Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(JETTY) >= 0) {
>> -                Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(ORION) >= 0) {
>> -                Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -                Debug.logInfo("Orion detected, using non-nested JspException", module);
>> -                nestjspexception = false;
>> -            } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
>> -                Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> +    private static UtilJ2eeCompat getInstance(ServletContext context) {
>> +        if (instance == null) {
>> +            synchronized (UtilJ2eeCompat.class) {
>> +                if (instance == null) {
>> +                    instance = new UtilJ2eeCompat(context);
>> +                }
>>             }
>> -
>> -            doFlushOnRenderValue = Boolean.valueOf(doflush);
>> -            useOutputStreamNotWriterValue = Boolean.valueOf(usestream);
>> -            useNestedJspException = Boolean.valueOf(nestjspexception);
>>         }
>> +        return instance;
>>     }
>> +
>> +    public static boolean useOutputStreamNotWriter(ServletContext context) {
>> +        return getInstance(context).useOutputStreamNotWriter;
>> +    }
>> +
>> }
>> 
>> 


Re: svn commit: r1622193 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
+1, we all know DCL is an antipattern

Jacques

Le 03/09/2014 15:38, Adrian Crum a écrit :
> The DCL design pattern will fail 90% of the time. I was able to prove this a while back when I removed the DCL pattern from the transaction factory 
> and connection factory.
>
> I recommend using an atomic reference.
>
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
>
> On 9/3/2014 10:10 AM, jacopoc@apache.org wrote:
>> Author: jacopoc
>> Date: Wed Sep  3 09:10:04 2014
>> New Revision: 1622193
>>
>> URL: http://svn.apache.org/r1622193
>> Log:
>> Greatly simplified the UtilJ2eeCompat class by removing a series of variables and methods no more in use.
>>   The class was not thread safe and this didn't cause particular issues probably just because all the threads using it were passing ServletContext 
>> with the same server info value (because they were associated to the same servlet container); it was however inefficient.
>>   Now it is thread safe using the DCL pattern which I usually tend to avoid but is useful here.
>>   However this class should probably go away but before taking a decision I will start a discussion in the dev list.
>>
>> Modified:
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java?rev=1622193&r1=1622192&r2=1622193&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java Wed Sep  3 09:10:04 2014
>> @@ -28,74 +28,60 @@ public class UtilJ2eeCompat {
>>
>>       public static final String module = UtilJ2eeCompat.class.getName();
>>
>> -    public static final String TOMCAT = "apache tomcat";
>> -    public static final String ORION = "orion";
>> -    public static final String RESIN = "resin";
>> -    public static final String REX_IP = "tradecity";
>> -    public static final String OC4J = "oracle";
>> -    public static final String JRUN = "jrun";
>> -    public static final String JETTY = "jetty";
>> -    public static final String WEBSPHERE = "websphere";
>> -
>> -    protected static Boolean doFlushOnRenderValue = null;
>> -    protected static Boolean useOutputStreamNotWriterValue = null;
>> -    protected static Boolean useNestedJspException = null;
>> -
>> -    public static boolean doFlushOnRender(ServletContext context) {
>> -        initCompatibilityOptions(context);
>> -        return doFlushOnRenderValue.booleanValue();
>> -    }
>> -
>> -    public static boolean useOutputStreamNotWriter(ServletContext context) {
>> -        initCompatibilityOptions(context);
>> -        return useOutputStreamNotWriterValue.booleanValue();
>> -    }
>> +    private static final String TOMCAT = "apache tomcat";
>> +    private static final String ORION = "orion";
>> +    private static final String REX_IP = "tradecity";
>> +    private static final String JRUN = "jrun";
>> +    private static final String JETTY = "jetty";
>> +    private static final String WEBSPHERE = "websphere";
>> +
>> +    private volatile static UtilJ2eeCompat instance;
>> +
>> +    private final boolean useOutputStreamNotWriter;
>> +
>> +    private UtilJ2eeCompat(ServletContext context) {
>> +        boolean usestream = true;
>> +        // if context is null use an empty string here which will cause the defaults to be used
>> +        String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
>> +
>> +        Debug.logInfo("serverInfo: " + serverInfo, module);
>> +
>> +        if (serverInfo.indexOf(TOMCAT) >= 0) {
>> +            Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(REX_IP) >= 0) {
>> +            Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(JRUN) >= 0) {
>> +            Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(JETTY) >= 0) {
>> +            Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(ORION) >= 0) {
>> +            Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> +            usestream = false;
>> +        } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
>> +            Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of 
>> response.getOutputStream", module);
>> +            usestream = false;
>> +        }
>>
>> -    public static boolean useNestedJspException(ServletContext context) {
>> -        initCompatibilityOptions(context);
>> -        return useNestedJspException.booleanValue();
>> +        useOutputStreamNotWriter = usestream;
>>       }
>>
>> -    protected static void initCompatibilityOptions(ServletContext context) {
>> -        // this check to see if we should flush is done because on most servers this
>> -        // will just slow things down and not solve any problems, but on Tomcat, Orion, etc it is necessary
>> -        if (useOutputStreamNotWriterValue == null || doFlushOnRenderValue == null) {
>> -            boolean doflush = true;
>> -            boolean usestream = true;
>> -            boolean nestjspexception = true;
>> -            // if context is null use an empty string here which will cause the defaults to be used
>> -            String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
>> -
>> -            Debug.logInfo("serverInfo: " + serverInfo, module);
>> -
>> -            if (serverInfo.indexOf(RESIN) >= 0) {
>> -                Debug.logInfo("Resin detected, disabling the flush on the region render from PageContext for better performance", module);
>> -                doflush = false;
>> -            } else if (serverInfo.indexOf(REX_IP) >= 0) {
>> -                Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(TOMCAT) >= 0) {
>> -                Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(JRUN) >= 0) {
>> -                Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(JETTY) >= 0) {
>> -                Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -            } else if (serverInfo.indexOf(ORION) >= 0) {
>> -                Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
>> -                usestream = false;
>> -                Debug.logInfo("Orion detected, using non-nested JspException", module);
>> -                nestjspexception = false;
>> -            } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
>> -                Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of 
>> response.getOutputStream", module);
>> -                usestream = false;
>> +    private static UtilJ2eeCompat getInstance(ServletContext context) {
>> +        if (instance == null) {
>> +            synchronized (UtilJ2eeCompat.class) {
>> +                if (instance == null) {
>> +                    instance = new UtilJ2eeCompat(context);
>> +                }
>>               }
>> -
>> -            doFlushOnRenderValue = Boolean.valueOf(doflush);
>> -            useOutputStreamNotWriterValue = Boolean.valueOf(usestream);
>> -            useNestedJspException = Boolean.valueOf(nestjspexception);
>>           }
>> +        return instance;
>>       }
>> +
>> +    public static boolean useOutputStreamNotWriter(ServletContext context) {
>> +        return getInstance(context).useOutputStreamNotWriter;
>> +    }
>> +
>>   }
>>
>>
>

Re: svn commit: r1622193 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java

Posted by Adrian Crum <ad...@sandglass-software.com>.
The DCL design pattern will fail 90% of the time. I was able to prove 
this a while back when I removed the DCL pattern from the transaction 
factory and connection factory.

I recommend using an atomic reference.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 9/3/2014 10:10 AM, jacopoc@apache.org wrote:
> Author: jacopoc
> Date: Wed Sep  3 09:10:04 2014
> New Revision: 1622193
>
> URL: http://svn.apache.org/r1622193
> Log:
> Greatly simplified the UtilJ2eeCompat class by removing a series of variables and methods no more in use.
>   The class was not thread safe and this didn't cause particular issues probably just because all the threads using it were passing ServletContext with the same server info value (because they were associated to the same servlet container); it was however inefficient.
>   Now it is thread safe using the DCL pattern which I usually tend to avoid but is useful here.
>   However this class should probably go away but before taking a decision I will start a discussion in the dev list.
>
> Modified:
>      ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java?rev=1622193&r1=1622192&r2=1622193&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilJ2eeCompat.java Wed Sep  3 09:10:04 2014
> @@ -28,74 +28,60 @@ public class UtilJ2eeCompat {
>
>       public static final String module = UtilJ2eeCompat.class.getName();
>
> -    public static final String TOMCAT = "apache tomcat";
> -    public static final String ORION = "orion";
> -    public static final String RESIN = "resin";
> -    public static final String REX_IP = "tradecity";
> -    public static final String OC4J = "oracle";
> -    public static final String JRUN = "jrun";
> -    public static final String JETTY = "jetty";
> -    public static final String WEBSPHERE = "websphere";
> -
> -    protected static Boolean doFlushOnRenderValue = null;
> -    protected static Boolean useOutputStreamNotWriterValue = null;
> -    protected static Boolean useNestedJspException = null;
> -
> -    public static boolean doFlushOnRender(ServletContext context) {
> -        initCompatibilityOptions(context);
> -        return doFlushOnRenderValue.booleanValue();
> -    }
> -
> -    public static boolean useOutputStreamNotWriter(ServletContext context) {
> -        initCompatibilityOptions(context);
> -        return useOutputStreamNotWriterValue.booleanValue();
> -    }
> +    private static final String TOMCAT = "apache tomcat";
> +    private static final String ORION = "orion";
> +    private static final String REX_IP = "tradecity";
> +    private static final String JRUN = "jrun";
> +    private static final String JETTY = "jetty";
> +    private static final String WEBSPHERE = "websphere";
> +
> +    private volatile static UtilJ2eeCompat instance;
> +
> +    private final boolean useOutputStreamNotWriter;
> +
> +    private UtilJ2eeCompat(ServletContext context) {
> +        boolean usestream = true;
> +        // if context is null use an empty string here which will cause the defaults to be used
> +        String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
> +
> +        Debug.logInfo("serverInfo: " + serverInfo, module);
> +
> +        if (serverInfo.indexOf(TOMCAT) >= 0) {
> +            Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> +            usestream = false;
> +        } else if (serverInfo.indexOf(REX_IP) >= 0) {
> +            Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> +            usestream = false;
> +        } else if (serverInfo.indexOf(JRUN) >= 0) {
> +            Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> +            usestream = false;
> +        } else if (serverInfo.indexOf(JETTY) >= 0) {
> +            Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> +            usestream = false;
> +        } else if (serverInfo.indexOf(ORION) >= 0) {
> +            Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> +            usestream = false;
> +        } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
> +            Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> +            usestream = false;
> +        }
>
> -    public static boolean useNestedJspException(ServletContext context) {
> -        initCompatibilityOptions(context);
> -        return useNestedJspException.booleanValue();
> +        useOutputStreamNotWriter = usestream;
>       }
>
> -    protected static void initCompatibilityOptions(ServletContext context) {
> -        // this check to see if we should flush is done because on most servers this
> -        // will just slow things down and not solve any problems, but on Tomcat, Orion, etc it is necessary
> -        if (useOutputStreamNotWriterValue == null || doFlushOnRenderValue == null) {
> -            boolean doflush = true;
> -            boolean usestream = true;
> -            boolean nestjspexception = true;
> -            // if context is null use an empty string here which will cause the defaults to be used
> -            String serverInfo = context == null ? "" : context.getServerInfo().toLowerCase();
> -
> -            Debug.logInfo("serverInfo: " + serverInfo, module);
> -
> -            if (serverInfo.indexOf(RESIN) >= 0) {
> -                Debug.logInfo("Resin detected, disabling the flush on the region render from PageContext for better performance", module);
> -                doflush = false;
> -            } else if (serverInfo.indexOf(REX_IP) >= 0) {
> -                Debug.logInfo("Trade City RexIP detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> -                usestream = false;
> -            } else if (serverInfo.indexOf(TOMCAT) >= 0) {
> -                Debug.logInfo("Apache Tomcat detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> -                usestream = false;
> -            } else if (serverInfo.indexOf(JRUN) >= 0) {
> -                Debug.logInfo("JRun detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> -                usestream = false;
> -            } else if (serverInfo.indexOf(JETTY) >= 0) {
> -                Debug.logInfo("Jetty detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> -                usestream = false;
> -            } else if (serverInfo.indexOf(ORION) >= 0) {
> -                Debug.logInfo("Orion detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> -                usestream = false;
> -                Debug.logInfo("Orion detected, using non-nested JspException", module);
> -                nestjspexception = false;
> -            } else if (serverInfo.indexOf(WEBSPHERE) >= 0) {
> -                Debug.logInfo("IBM Websphere Application Server detected, using response.getWriter to write text out instead of response.getOutputStream", module);
> -                usestream = false;
> +    private static UtilJ2eeCompat getInstance(ServletContext context) {
> +        if (instance == null) {
> +            synchronized (UtilJ2eeCompat.class) {
> +                if (instance == null) {
> +                    instance = new UtilJ2eeCompat(context);
> +                }
>               }
> -
> -            doFlushOnRenderValue = Boolean.valueOf(doflush);
> -            useOutputStreamNotWriterValue = Boolean.valueOf(usestream);
> -            useNestedJspException = Boolean.valueOf(nestjspexception);
>           }
> +        return instance;
>       }
> +
> +    public static boolean useOutputStreamNotWriter(ServletContext context) {
> +        return getInstance(context).useOutputStreamNotWriter;
> +    }
> +
>   }
>
>