tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject [Bug 56166] Suggestions for exception handling (avoid potential bugs)
Date Thu, 20 Feb 2014 22:32:25 GMT
https://issues.apache.org/bugzilla/show_bug.cgi?id=56166

--- Comment #2 from Ding Yuan <yuan@eecg.utoronto.ca> ---
Thanks a lot for the feedbacks and sorry for those non-issues. For what it is
worth, I did a bit more digging to follow-up on your comments regarding to the
effects of the following cases:

">ApplicationFilterFactory,
>DefaultInstanceManager,
>NamingContextListener
>StandardServer
>StandardService
-0... these are likely already have an effect elsewhere (e.g. Tomcat not
starting) or are logged elsewhere."

> ApplicationFilterFactory: 
   if (filterConfig == null) {
       // FIXME - log configuration problem
       continue;
   }
The effect is that the filterConfig won't added into filterChain and the
function will return. It's not obvious how the caller handles this. At least
each time when filterConfig==null occurs during iterating through it would be
lost.

>DefaultInstanceManager
The effect will be a null pointer dereference. If the exception occurs, it will
cause getField() to return null, which is passed into lookupFieldResource(..),
which will dereference the null pointer and cause a null pointer dereference
exception. I wonder if this is the desired behaviour.

>NamingContextListener
The effect will be a null pointer dereference. If the exception occurs,
namingContext will be null and there would be pointer dereference 10 lines
later: namingContext.setExceptionOnFailedWrite(..);

>StandardServer
>StandardService
Sorry I am not sure about the effect. When Service.start() throws
LifecycleException and it is swallowed, addservice seems will return and it is
not obvious how the code will eventually interpret this error.

">+0. Catching a Throwable isn't good. I'd be better to fix this."
In case it helps, there are a few more cases where a throwable is caught and
ignored:

  Line: 202, File: "org/apache/catalina/storeconfig/StoreLoader.java"

193:         try {
194:             String configUrl = getConfigUrl();
195:             if (configUrl != null) {
196:                 is = (new URL(configUrl)).openStream();
197:                 if (log.isInfoEnabled())
198:                     log.info("Find registry server-registry.xml from
system property at url "
199:                             + configUrl);
200:                 registryResource = new URL(configUrl);
201:             }
202:         } catch (Throwable t) {
203:             // Ignore
204:         }
=====
 Line: 46, File: "org/apache/juli/logging/DirectJDKLog.java"
44:             try {
45:                 Class.forName(SIMPLE_CFG).newInstance();
46:             } catch( Throwable t ) {
47:             } 

but it seems this error is not important. 
====
  Line: 60, File: "org/apache/juli/logging/DirectJDKLog.java"

48:             try {
49:                 Formatter
fmt=(Formatter)Class.forName(System.getProperty(FORMATTER,
SIMPLE_FMT)).newInstance();
50:                 // it is also possible that the user modified
jre/lib/logging.properties -
51:                 // but that's really stupid in most cases
52:                 Logger root=Logger.getLogger("");
53:                 Handler handlers[]=root.getHandlers();
54:                 for( int i=0; i< handlers.length; i++ ) {
55:                     // I only care about console - that's what's used in
default config anyway
56:                     if( handlers[i] instanceof  ConsoleHandler ) {
57:                         handlers[i].setFormatter(fmt);
58:                     }
59:                 }
60:             } catch( Throwable t ) {
61:                 // maybe it wasn't included - the ugly default will be
used.
62:             }
=====
  Line: 424, File: "org/apache/tomcat/util/modeler/Registry.java"

419:         try {
420:             if( getMBeanServer().isRegistered(oname)) {
421:                 getMBeanServer().unregisterMBean(oname);
422:             }
423:         } catch( Throwable t ) {
424:             log.error( "Error unregistering mbean ", t);
425:         }


Please let me know if there is anything more I could help in fixing these.
Thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


Mime
View raw message