RE: XalanJ2 BUG - quiet swallowing of exceptions - with proposed fix attached

[Patrick Moore <pa...@rioport.com>]

Slight revised version....

> -----Original Message-----
> From: Patrick Moore [mailto:patrickm@rioport.com]
> Sent: Friday, February 23, 2001 6:17 PM
> To: 'xalan-dev@xml.apache.org'
> Subject: XalanJ2 BUG - quiet swallowing of exceptions - with proposed
> fix attached
> 
> 
> Hi there -- 
> 
> I have found some bad behavior.
> 
> Repro:
> 
> 1. Throw a RuntimeException from a ContentHandler method (I 
> noticed this
> problem while in startElement() ). 
> 
> 2. Make sure this exception has a message.
> 
> 3. This exception will never get printed and the program will 
> quietly die.
> 
> Now here is the puzzling part of the bug. The code in
> TransformerImpl.postExceptionFromThread(Exception e) 
> [lines2829:2871] is
> very puzzling. First, there is the "Nicola Brown" comment 
> about there being
> some sort of problem. What is also odd about this is that in 
> every place in
> TransformerImpl, m_reportInPostExceptionFromThread is never 
> true. Is this
> old code that can be discarded?
> 
> Then there is the later comment "  // should have already 
> been reported via
> the error handler?". Finally, to really add to the confusion, 
> there is the
> way the exception is treated in the synchronized block where if the
> exception's message is null then the exception is printed.
> 
> I have included a proposed fix. But someone more in the know needs to
> examine this and see 1) if the "Nicola Brown" reference is 
> still valid;
> and/or 2) should all exception be printed. Note too that this 
> change means
> that a checked exception with no message will no longer be 
> printed which I
> am not at all certain is correct.
> 
> On a minor note: I removed some catches that simply rethrew 
> the exception
> 
> -Patrick Moore-
> 
> excerpt:
> 
> void postExceptionFromThread(Exception e)
> {
> // Commented out in response to problem reported by Nicola Brown
> <Ni...@jacobsrimell.com>
> //    if(m_reportInPostExceptionFromThread)
> //    {
> //      // Consider re-throwing the exception if this flag is set.
> //      e.printStackTrace();
> //    }
> 
>     if (m_inputContentHandler instanceof SourceTreeHandler)
> 	{
> 	  SourceTreeHandler sth = (SourceTreeHandler) 
> m_inputContentHandler;
> 
> 	  sth.setExceptionThrown(e);
> 	}
> 	ContentHandler ch = getContentHandler();
> 	if(ch instanceof SourceTreeHandler)
> 	{
> 	  SourceTreeHandler sth = (SourceTreeHandler) ch;
> 	
> ((TransformerImpl)(sth.getTransformer())).postExceptionFromThread(e);
> 	}
> 
> 	m_isTransformDone = true;
> 	m_exceptionThrown = e;
> 	;  // should have already been reported via the error handler?
> 
> 	synchronized (this)
> 	{
> 	  String msg = e.getMessage();
> 
> 	  // System.out.println(e.getMessage());
> 	  notifyAll();
> 
> 	  if (null == msg)
> 	  {
> 
> 		// m_throwNewError = false;
> 		e.printStackTrace();
> 	  }
> 
> 	  // throw new org.apache.xml.utils.WrappedRuntimeException(e);
> 	}
>  }
> 
>