tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sean McFadden <s...@wingateweb.com>
Subject Re: [PATCH] Connection close bug fixed, logging improved
Date Fri, 16 Jun 2000 17:48:06 GMT
Will calls to ServletContext.log(String myMessage, Throwable myThrowable) print the stack trace
for myThrowable now?

Alex Chaffee wrote:

> Here's my patch.
>
> It fixes a bug whereby an overzealous exception handler needlessly
> closes an endpoint, in response to a normal socket exception thrown if
> the client closes the connection while still inside
> ServerSocket.accept().
>
> It also cleans up logging a bit, to wit:
>
> * removes hardcoded setCustomOutput(true) and
> setVerbosityLevel(Logger.INFORMATION), so server.xml is honored
>
> * establishes a LogHelper object that is instantiated by each log
> client; this object serves to make log behavior more consistent across
> the program. It prints a prefix (usually the name of the class calling
> the logger), and also outputs to stderr or stdout if the logger is
> unavailable, to facilitate out-of-tomcat use.
>
> * adds setLogger method to ServerConnector interface, again to
> facilitate out-of-tomcat use.
>
> * clean up several catch blocks and System.out statements.
>
> I rely (once) on the static Logger.getLogger() method, and hardcode
> "tc_log" in places, but I think that's actually fairly clean.  Feel
> free to criticize my design.
>
> If you're having the stray SocketException problem (e.g. Mark
> Brouwer), you can focus on my changes to HttpConnectionHandler.java
> and PoolTcpEndpoint.java and ignore the logging stuff.
>
> The reason I merged them is just that, because logging was
> inconsistent, it was hard for me to track down the socket bug, so I
> ended up fixing them both at the same time.
>
>  - Alex
>
> --
> Alex Chaffee                       mailto:alex@jguru.com
> jGuru - Java News and FAQs         http://www.jguru.com/alex/
> Creator of Gamelan                 http://www.gamelan.com/
> Founder of Purple Technology       http://www.purpletech.com/
> Curator of Stinky Art Collective   http://www.stinky.com/
>
> Index: ./org/apache/tomcat/service/http/HttpConnectionHandler.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/service/http/HttpConnectionHandler.java,v
> retrieving revision 1.25
> diff -r1.25 HttpConnectionHandler.java
> 72a73
> > import org.apache.tomcat.logging.*;
> 210,211d210
> <           } catch(java.net.SocketException ex) {
> <               // do nothing - same
> 213,217c212,230
> <           //      System.out.print("5");
> <       } catch (Exception e) {
> <           contextM.log( "Error reading request " + e.getMessage());
> <           e.printStackTrace();
> <       } finally {
> ---
> >       }
> >       catch(java.net.SocketException e) {
> >           // SocketExceptions are normal
> >           contextM.doLog( "SocketException reading request, ignored", e, Logger.INFORMATION);
> >       }
> >       catch (java.io.IOException e) {
> >           // IOExceptions are normal
> >           contextM.doLog( "IOException reading request, ignored", e, Logger.INFORMATION);
> >       }
> >       // Future developers: if you discover any other
> >       // rare-but-nonfatal exceptions, catch them here, and log
> >       // with INFORMATION level.
> >       catch (Throwable e) {
> >           // any other exception or error is odd. Here we log it
> >           // with "ERROR" level, so it will show up even on
> >           // less-than-verbose logs.
> >           contextM.doLog( "Error reading request, ignored", e, Logger.ERROR);
> >       }
> >       finally {
> Index: ./org/apache/tomcat/core/Context.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/core/Context.java,v
> retrieving revision 1.93
> diff -r1.93 Context.java
> 700,701d699
> <               csLog.setCustomOutput("true");
> <               csLog.setVerbosityLevel(Logger.INFORMATION);
> Index: ./org/apache/tomcat/core/ContextManager.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/core/ContextManager.java,v
> retrieving revision 1.89
> diff -r1.89 ContextManager.java
> 1040c1040
> <     Logger cmLog = null;
> ---
> >     LogHelper loghelper = new LogHelper("tc_log", "ContextManager");
> 1042c1042,1043
> <
> ---
> >     // Not used, except in server.xml, and usage is unclear -- should
> >     // we kill it? Looks very obsolete.
> 1072c1073
> <       doLog( "CM: " + msg );
> ---
> >       loghelper.log(msg);
> 1076c1077
> <       doLog( msg, null);
> ---
> >       loghelper.log(msg);
> 1080,1087c1081,1082
> <       if (firstLog == true) {
> <           cmLog = Logger.getLogger("tc_log");
> <           if( cmLog!= null ) {
> <               cmLog.setCustomOutput("true");
> <               cmLog.setVerbosityLevel(Logger.INFORMATION);
> <           }
> <           firstLog = false;
> <       }
> ---
> >       loghelper.log(msg, t);
> >     }
> 1089,1096c1084,1085
> <       if (cmLog != null) {
> <           cmLog.log(msg + "\n", t, Logger.INFORMATION);
> <           // XXX \n should be added to logger, portable
> <       } else {
> <           System.out.println(msg);
> <           if( t!=null )
> <               t.printStackTrace( System.out );
> <       }
> ---
> >     public final void doLog(String msg, Throwable t, int level) {
> >       loghelper.log(msg, t, level);
> Index: ./org/apache/tomcat/core/ServerConnector.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/core/ServerConnector.java,v
> retrieving revision 1.3
> diff -r1.3 ServerConnector.java
> 105a106,109
> >
> >     /** Set a org.apache.tomcat.logging.Logger explicitly
> >      */
> >     public void setLogger( org.apache.tomcat.logging.Logger logger );
> Index: ./org/apache/tomcat/logging/TomcatLogger.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/logging/TomcatLogger.java,v
> retrieving revision 1.6
> diff -r1.6 TomcatLogger.java
> 115,118c115,129
> <           // custom output - do nothing
> <           if( TomcatLogger.this.custom )
> <               return (t==null) ? message : message + " " + throwableToString( t
);
> <
> ---
> >           // custom output, now with timestamp
> >           if( TomcatLogger.this.custom ) {
> >               StringBuffer val = new StringBuffer();
> >               if (TomcatLogger.this.timeStamp) {
> >                   val.append(new Date(date).toString());
> >                   val.append(" ");
> >               }
> >               val.append(message);
> >               if (t != null) {
> >                   val.append(" ");
> >                   val.append(throwableToString( t ));
> >               }
> >               return val.toString();
> >           }
> >
> Index: ./org/apache/tomcat/service/JNIEndpointConnector.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/service/JNIEndpointConnector.java,v
> retrieving revision 1.2
> diff -r1.2 JNIEndpointConnector.java
> 69a70
> > import org.apache.tomcat.logging.*;
> 93c94
> <     public static void setEndpoinet(JNIEndpoint ep)
> ---
> >     public static void setEndpoint(JNIEndpoint ep)
> 133a135,144
> >
> >     private LogHelper loghelper = new LogHelper("tc_log", "JNIEndpointConnector");
> >
> >     /**
> >      * Set a logger explicitly.
> >      **/
> >     public void setLogger( Logger logger ) {
> >       loghelper.setLogger(logger);
> >     }
> >
> Index: ./org/apache/tomcat/service/LocalStrings.properties
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/service/LocalStrings.properties,v
> retrieving revision 1.1.1.1
> diff -r1.1.1.1 LocalStrings.properties
> 11a12
> > endpoint.err.nonfatal=Endpoint {0} ignored exception: {1}
> Index: ./org/apache/tomcat/service/PoolTcpConnector.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/service/PoolTcpConnector.java,v
> retrieving revision 1.6
> diff -r1.6 PoolTcpConnector.java
> 65a66
> > import org.apache.tomcat.logging.*;
> 119a121,122
> >     private LogHelper loghelper = new LogHelper("tc_log", "PoolTcpConnector");
> >
> 188c191,192
> <       System.out.println("Starting " + classN + " on " + port);
> ---
> >
> >       loghelper.log("Starting " + classN + " on " + port);
> 247c251,252
> <       if( debug > 0 ) log( "setAttribute( " + prop + " , " + value + ")");
> ---
> >       if( debug > 0 )
> >           loghelper.log( "setAttribute( " + prop + " , " + value + ")");
> 248a254
> >       try {
> 292a299,302
> >       }
> >       catch (Exception e) {
> >           loghelper.log("setAttribute: " +prop + "=" + value, e, Logger.ERROR);
> >       }
> 298a309,317
> >     /**
> >      * Set a logger explicitly. Note that setLogger(null) will not
> >      * necessarily redirect log output to System.out; if there is a
> >      * "tc_log" logger it will default back to using it.
> >      **/
> >     public void setLogger( Logger logger ) {
> >       loghelper.setLogger(logger);
> >     }
> >
> 301,308c320,326
> <     private static TcpConnectionHandler string2ConnectionHandler( String val) {
> <       try {
> <           Class chC=Class.forName( val );
> <           return (TcpConnectionHandler)chC.newInstance();
> <       } catch( Exception ex) {
> <           ex.printStackTrace();
> <       }
> <       return null;
> ---
> >
> >     // now they just throw exceptions, which are caught and logged by
> >     // the caller
> >
> >     private static TcpConnectionHandler string2ConnectionHandler( String val) throws
ClassNotFoundException, IllegalAccessException, InstantiationException {
> >       Class chC=Class.forName( val );
> >       return (TcpConnectionHandler)chC.newInstance();
> 311,318c329,331
> <     private static ServerSocketFactory string2SocketFactory( String val) {
> <       try {
> <           Class chC=Class.forName( val );
> <           return (ServerSocketFactory)chC.newInstance();
> <       } catch( Exception ex) {
> <           ex.printStackTrace();
> <       }
> <       return null;
> ---
> >     private static ServerSocketFactory string2SocketFactory( String val) throws
ClassNotFoundException, IllegalAccessException, InstantiationException {
> >       Class chC=Class.forName( val );
> >       return (ServerSocketFactory)chC.newInstance();
> 321,326c334,335
> <     private static InetAddress string2Inet( String val) {
> <       try {
> <           return InetAddress.getByName( val );
> <       } catch( Exception ex) {
> <       }
> <       return null;
> ---
> >     private static InetAddress string2Inet( String val) throws UnknownHostException
{
> >       return InetAddress.getByName( val );
> 330,334c339
> <       try {
> <               return Integer.parseInt(val);
> <       } catch (NumberFormatException nfe) {
> <               return 0;
> <       }
> ---
> >       return Integer.parseInt(val);
> 337,339d341
> <     void log( String s ) {
> <       System.out.println("PoolTcpConnector: " + s );
> <     }
> Index: ./org/apache/tomcat/service/PoolTcpEndpoint.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/service/PoolTcpEndpoint.java,v
> retrieving revision 1.7
> diff -r1.7 PoolTcpEndpoint.java
> 68a69
> > import org.apache.tomcat.logging.*;
> 108a110,111
> >     private LogHelper loghelper = new LogHelper("tc_log", "PoolTcpEndpoint");
> >
> 125,126c128,133
> <     void log( String s ) {
> <       System.out.println("PoolTcpEndpoint: " + s );
> ---
> >     private void log( String msg ) {
> >       loghelper.log(msg);
> >     }
> >
> >     private void log(String msg, Throwable t, int level) {
> >       loghelper.log(msg, t, level);
> 254c261
> <           System.out.println("XXX Error - need pool !");
> ---
> >           log("XXX Error - need pool !", null, Logger.ERROR);
> 295a303,311
> >
> >           // TCP stacks can throw SocketExceptions when the client
> >           // disconnects.  We don't want this to shut down the
> >           // endpoint, so ignore it. Is there a more robust
> >           // solution?  Should we compare the message string to
> >           // "Connection reset by peer"?
> >
> >           // socket exceptions just after closing endpoint aren't
> >           // even logged
> 297,301c313,315
> <                       running = false;
> <                       String msg = sm.getString("endpoint.err.fatal",
> <                                                         serverSocket, e);
> <               e.printStackTrace(); // something very wrong happened - better know
what
> <                   System.err.println(msg);
> ---
> >               String msg = sm.getString("endpoint.err.nonfatal",
> >                                         serverSocket, e);
> >               log(msg, e, Logger.INFORMATION);
> 303c317,323
> <       } catch(Throwable e) {
> ---
> >
> >       }
> >
> >       // Future developers: if you identify any other nonfatal
> >       // exceptions, catch them here and log as above
> >
> >       catch(Throwable e) {
> 306,308c326,327
> <                                                 serverSocket, e);
> <           e.printStackTrace(); // something very wrong happened - better know what
> <           System.err.println(msg);
> ---
> >                                     serverSocket, e);
> >           log(msg, e, Logger.ERROR);
>
> ./org/apache/tomcat/logging/LogHelper.java
> ===================================================================
> /*
>  * ====================================================================
>  *
>  * The Apache Software License, Version 1.1
>  *
>  * Copyright (c) 1999 The Apache Software Foundation.  All rights
>  * reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>  * are met:
>  *
>  * 1. Redistributions of source code must retain the above copyright
>  *    notice, this list of conditions and the following disclaimer.
>  *
>  * 2. Redistributions in binary form must reproduce the above copyright
>  *    notice, this list of conditions and the following disclaimer in
>  *    the documentation and/or other materials provided with the
>  *    distribution.
>  *
>  * 3. The end-user documentation included with the redistribution, if
>  *    any, must include the following acknowlegement:
>  *       "This product includes software developed by the
>  *        Apache Software Foundation (http://www.apache.org/)."
>  *    Alternately, this acknowlegement may appear in the software itself,
>  *    if and wherever such third-party acknowlegements normally appear.
>  *
>  * 4. The names "The Jakarta Project", "Tomcat", and "Apache Software
>  *    Foundation" must not be used to endorse or promote products derived
>  *    from this software without prior written permission. For written
>  *    permission, please contact apache@apache.org.
>  *
>  * 5. Products derived from this software may not be called "Apache"
>  *    nor may "Apache" appear in their names without prior written
>  *    permission of the Apache Group.
>  *
>  * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
>  * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
>  * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>  * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
>  * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>  * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>  * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
>  * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
>  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>  * SUCH DAMAGE.
>  * ====================================================================
>  *
>  * This software consists of voluntary contributions made by many
>  * individuals on behalf of the Apache Software Foundation.  For more
>  * information on the Apache Software Foundation, please see
>  * <http://www.apache.org/>.
>  *
>  */
> package org.apache.tomcat.logging;
>
> import java.io.PrintStream;
>
> /**
>  * Wrapper for Logger. It has a preferred log name to write to; if it
>  * can't find a log with that name, it outputs to either System.out or
>  * System.err depending on the message's warning level.  Also prepends
>  * a descriptive name to each message (usually the name of the calling
>  * class), so it's easier to identify the source.<p>
>  *
>  * Intended for use by client classes to make it easy to do reliable,
>  * consistent logging behavior, even if you don't necessarily have a
>  * context, or if you haven't registered any log files yet.
>  *
>  * @author Alex Chaffee [alex@jguru.com]
>  **/
>
> public class LogHelper {
>
>     private String logname;
>     private String prepend;
>     private Logger logger;
>
>     public LogHelper(String logname, String prepend)
>     {
>         this.logname = logname;
>         this.prepend = prepend;
>     }
>
>     public Logger getLogger() {
>         return logger;
>     }
>
>     /**
>      * Set a logger explicitly. Note that setLogger(null) will not
>      * necessarily redirect log output to System.out; if there is a
>      * logname logger it will default back to using it.
>      **/
>     public void setLogger(Logger logger) {
>         this.logger = logger;
>     }
>
>     /**
>      * Logs the message with level INFORMATION
>      **/
>     public void log(String msg)
>     {
>         log(msg, null, Logger.INFORMATION);
>     }
>
>     /**
>      * Logs the Throwable with level ERROR (assumes an exception is
>      * trouble)
>      **/
>     public void log(String msg, Throwable t)
>     {
>         log(msg, t, Logger.ERROR);
>     }
>
>     /**
>      * Logs the message and Throwable to its logger or, if logger not
>      * found, to System.err or System.out (depending on level).
>      **/
>     public void log(String msg, Throwable t, int level)
>     {
>         if (prepend != null) {
>             msg = prepend + ": " + msg;
>         }
>
>         if (logger == null) {
>             if (logname != null)
>                 logger = Logger.getLogger(logname);
>         }
>
>         if (logger == null) {
>             // if it's still null, send to stderr or stdout
>             // ??? maybe should just send everything to one or the other
>             PrintStream out = (level <= Logger.WARNING) ? System.err : System.out;
>             out.println(msg);
>             if (t != null)
>                 t.printStackTrace(out);
>         }
>         else {
>             msg += "\n";
>             logger.log(msg, t, level);
>         }
>     }
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org


Mime
View raw message