[
https://issues.apache.org/jira/browse/WICKET-4600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14135767#comment-14135767
]
Andrea Del Bene commented on WICKET-4600:
-----------------------------------------
Hi,
I've also noted this kind of inconsistency looking at the code of
AbstractMarkupParser. AFAIU, most of the implementations of IResourceStream
maintain an inner state and method close() must always be invoked after we have
consumed the inputStream. In my opinion removing close() is quite tricky since
we would end up having reasource streams in an inconsistent state. Instead, as
Martin said, we should consider to fix all places in Wicket where it should be
used.
> Remove IResourceStream.close()
> ------------------------------
>
> Key: WICKET-4600
> URL: https://issues.apache.org/jira/browse/WICKET-4600
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 6.0.0-beta2
> Reporter: Jesse Long
>
> The IResourceStream.close() method is designed to allow the IResourceStream
> implementations to destroy data over and above the InputStream.
> If it was just the InputStream, we would not need the method, we could just
> call close() on the InputStream.
> The problem is, almost none of the wicket code correctly calls
> IResourceStream.close(). Often, IResourceStream.getInputStream() is called,
> often the InputStream is then closed, but IResourceStream.close() is not
> called.
> Also, it is not clear at which point IResourceStream.close() needs to be
> called. Possibly only after an InputStream is created, in which case, why
> does InputStream.close() not suffice? Otherwise, we should call
> IResourceStream.close() every time we use it - this very certainly does not
> happen.
> I propose to remove this method to save wicket the effort. I really doubt
> there is much real need to clean anything up in the close() method.
> IResourceStream implementation should retain enough information to create an
> InputStream, and create an InputStream method in the getInputStream() method,
> and close any no-longer needed resources in the getInputStream() method.
> Consumers of the IResourceStream can just close the InputStream and be done
> with it.
> Here is some discussion about some of the dodgey uses of
> IResourceStream.close in wicket ATM:
> AbstractMarkupParser calls IResourceInputStream.getInputStream(), but never
> calls IResourceStream.close().
> ContextRelativeResource call IResourceStream.getInputStream(), but never
> calls IResourceStream.close().
> ResourceStreamResource calls close() on the IResourceResponse returned from
> internalGetResourceStream(). However, it is possible that a new
> IResourceStream is dynamically created each time internalGetResourceStream is
> called, in which case ResourceStreamResponse closes the incorrect
> IResourceStream. Live would be easier if it only had to close the InputStream.
> MessageDigestResourceVersion calls IResourceStream.getInputStream(), but
> never calls IResourceStream.close().
> ConcatBundleResource calls IResourceStream.getInputStream(), but never calls
> IResourceStream.close(). (ConcatBundleResource also does stupid things with
> ByteArrayInputStream btw).
> XSLTResourceStream calls (in its constructor)
> IResourceStream.getInputStream() but never calls IResourceStream.close().
> XSLTResourceStream and its ZipDirectoryResourceStream friend really want to
> be deleted.
> Also, there are many many usages of IResourceStream where close() is not
> called, but getInputStream() is not called either.
> Wont mind working on a patch, after some direction from wicket devs.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
