Interesting. Seems like correct analysis.
I think the actual fix, however, would be to ad a handleFault method to the
AttachmentOutIntercept. On a fault/exception, it would unwind the chain into
there where it can clean itself up, which can include closing those streams.
Care to file a JIRA (and submit a patch. :-) )
Dan
On Tuesday 10 August 2010 10:31:55 am Martin Renner wrote:
> Hi Daniel,
>
> thank you for your response. The idea with the wrapped stream is great,
> because it won't scatter the code for opening and closing of the resources
> (like it would be the case with an interceptor).
>
> However, as far as I understood the code (CXF 2.2.9), there is a possible
> resource leak in the code for the attachment handling:
>
> AttachmentOutInterceptor (the EndingInterceptor) calls
> AttachmentSerializer.writeAttachments(). This method iterates over all
> attachments and calls "a.getDataHandler().writeTo(out)".
>
> Let's have a look at "DataHandler.writeTo(OutputStream os)":
>
> InputStream is = getInputStream();
> try {
> int count;
> while ((count = is.read(buffer)) != -1)
> os.write(buffer, 0, count);
> } finally {
> is.close();
> }
>
> That means, that DataHandler will always close the InputStream of the
> current attachment (which has been opened by the service implementation).
>
> But there are two problems:
>
> 1. Let's assume that there are 5 attachments and "writeTo" of the first
> attachment throws an IOException. No one will close the input streams of
> the remaining 4 attachments.
>
> 2. Let's assume that an exception was thrown somewhere between the service
> implementation (in fact the line where the service implementation opens
> the stream) and
> AttachmentSerializer.writeAttachments(). In this case too, no one will
> close the streams of the attachments.
>
> IMHO, there should be an interceptor in the OutFaultInterceptor chain which
> iterates over all attachments and calls
> "a.getDataHandler().getInputStream().close()" on each one.
>
>
> Kind Regards,
> Martin
--
Daniel Kulp
[email protected]
http://dankulp.com/blog
