tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Chaffee <g...@edamame.stinky.com>
Subject [PATCH] Connection close bug fixed, logging improved
Date Thu, 15 Jun 2000 18:27:58 GMT

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);
	}
    }
}

Mime
View raw message