You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2010/01/09 05:00:06 UTC

svn commit: r897380 - /tomcat/trunk/java/org/apache/juli/FileHandler.java

Author: kkolinko
Date: Sat Jan  9 03:59:59 2010
New Revision: 897380

URL: http://svn.apache.org/viewvc?rev=897380&view=rev
Log:
Followup for r816252/r891328
Allow to disable buffering in JULI FileHandler
The previous implementation did not work as expected because of buffering performed by OutputStreamWriter

Modified:
    tomcat/trunk/java/org/apache/juli/FileHandler.java

Modified: tomcat/trunk/java/org/apache/juli/FileHandler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/FileHandler.java?rev=897380&r1=897379&r2=897380&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/juli/FileHandler.java (original)
+++ tomcat/trunk/java/org/apache/juli/FileHandler.java Sat Jan  9 03:59:59 2010
@@ -145,7 +145,16 @@
         try {
             PrintWriter writer = this.writer;
             if (writer!=null) {
-                writer.write(result);
+                if (bufferSize > 0) {
+                    writer.write(result);
+                } else {
+                    synchronized (this) {
+                        // OutputStreamWriter performs buffering inside its StreamEncoder,
+                        // and so to run without a buffer we have to flush explicitly
+                        writer.write(result);
+                        writer.flush();
+                    }
+                }
             } else {
                 reportError("FileHandler is closed or not yet initialized, unable to log ["+result+"]", null, ErrorManager.WRITE_FAILURE);
             }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r897380 - /tomcat/trunk/java/org/apache/juli/FileHandler.java

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
On 01/08/2010 09:00 PM, kkolinko@apache.org wrote:
> Author: kkolinko
> Date: Sat Jan  9 03:59:59 2010
> New Revision: 897380
>
> URL: http://svn.apache.org/viewvc?rev=897380&view=rev
> Log:
> Followup for r816252/r891328
> Allow to disable buffering in JULI FileHandler
> The previous implementation did not work as expected because of buffering performed by OutputStreamWriter
>
> Modified:
>      tomcat/trunk/java/org/apache/juli/FileHandler.java
>
> Modified: tomcat/trunk/java/org/apache/juli/FileHandler.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/FileHandler.java?rev=897380&r1=897379&r2=897380&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/juli/FileHandler.java (original)
> +++ tomcat/trunk/java/org/apache/juli/FileHandler.java Sat Jan  9 03:59:59 2010
> @@ -145,7 +145,16 @@
>           try {
>               PrintWriter writer = this.writer;
>               if (writer!=null) {
> -                writer.write(result);
> +                if (bufferSize>  0) {
> +                    writer.write(result);
> +                } else {
> +                    synchronized (this) {
> +                        // OutputStreamWriter performs buffering inside its StreamEncoder,
> +                        // and so to run without a buffer we have to flush explicitly
> +                        writer.write(result);
> +                        writer.flush();
> +                    }
> +                }
>               } else {
>                   reportError("FileHandler is closed or not yet initialized, unable to log ["+result+"]", null, ErrorManager.WRITE_FAILURE);
>               }
>    

can you explain this to me?

we already have
OutputStream os = bufferSize>0?new BufferedOutputStream(fos,bufferSize):fos;

And what you're doing here is forcing a possibly disk flush and yet 
another synchronization

you could limit the sync block too

+                writer.write(result);
+                if (bufferSize<= 0) {
+                    synchronized (this) {
+                        // OutputStreamWriter performs buffering inside its StreamEncoder,
+                        // and so to run without a buffer we have to flush explicitly
+                        writer.flush();
+                    }
+                }

but why synchronized(this) ? output streams are already synchronized. 
This just doesn't seem right

Filip


>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>    


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org