tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: JAXBContext leaks memory
Date Mon, 17 May 2010 20:20:19 GMT
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark,

I know you posted this a while ago, but I have some comments:

On 5/4/2010 9:10 AM, Mark Shifman wrote:
> 	public <T> T getNextElement(String theElement, String elementAfter, Class <T>clazz)
{
> 		String elname = "";
> 		T h = null;
> 		try {
> 			while(reader.hasNext()){
> 			  if(reader.peek().isStartElement()){
> 				 elname = reader.peek().asStartElement().getName().getLocalPart();
> 				 if(elname.equals(theElement)){
> 			  	       h= u.unmarshal(reader, clazz).getValue();
> 			  	       return h;
> 				 }
> 			  } else if(reader.peek().isEndElement()){
> 					elname = reader.peek().asEndElement().getName().getLocalPart();
> 					if(elname.equals(elementAfter)){
> 						return h;
> 					}
> 				}
> 
> 			  reader.nextEvent();
> 			}
> 		} catch (XMLStreamException e) {
> 			throw new RuntimeException(e);
> 		} catch (JAXBException e) {
> 			throw new RuntimeException(e);
> 		}
> 		return h;
>     }

You could put the "return h" above the exception handlers to make it
clear that the try block must run to completion in order to return a
value. That's just a matter of taste, though.

> It also has a close method to clean up after I have gotten all the elements.
> 	public void close(){
> 		try {
> 			reader.close();
> 		} catch (XMLStreamException e) {
> 			//quietly

Don't do this: at least log the exception to stdout, or you may miss
something important at some point.

> 		}
> 		IOUtils.closeQuietly(jxb_in);
> 		u=null;
> 	}

You might want to put the call to IOUtils.closeQuietly into a finally
block: a RuntimeException could skip the call to your "close" method.

Whenever there's a close method available, you should call it.
Whenever you call it, you should probably call it in a finally blocks.

>>> 	private InputStream jxb_in;
>>>
>>> 	public static JAXBMascot getInstance(InputStream in) {
>>> 		JAXBMascot m = new JAXBMascot();
>>> 		try {
>>> 			m.setJxb_in(in);
>>> 			m.setReader(XMLInputFactory.newInstance().createXMLEventReader(in));
>>> 		} catch (Exception e) {
>>> 			log.fatal("error getting JAXBMascot instance");
>>> 			IOUtils.closeQuietly(in);
>>> 			throw new RuntimeException(e);
>>> 		}

Should you close even if no Exception has been thrown? The above does
not cover Error conditions, for example. Perhaps you wanted to catch
Throwable instead?

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvxpQMACgkQ9CaO5/Lv0PCyOgCfabMgxsAJzNplBsspeQnn83i2
KSwAn36FoEeFGJIQppwTpD5Ju338qIVX
=HAkp
-----END PGP SIGNATURE-----

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


Mime
View raw message