[
https://issues.apache.org/jira/browse/JCRVLT-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16897824#comment-16897824
]
Konrad Windszus edited comment on JCRVLT-349 at 8/1/19 7:36 AM:
----------------------------------------------------------------
bq. 2) I can't remember why the input stream is copied. but the
ByteArrayInputStream is not closed, as it will be freed automatically. I don't
think the config it used much elsewhere, so Ok not to close it.
Although not closing the ByteArrayInputStream does not have a negative impact
in fact the original input stream is closed. Keep that or change it?
bq. 3) ok. I don't think the config is used much elsewhere, so Ok to not close
it.
Are you proposing to change the behaviour?
bq. BTW, you also needs to consider the OutputStreams
Actually all {{save...}} methods in DefaultMetaInf don't close the temporary
FileInputStream, i.e. all those methods leak file handles.
was (Author: kwin):
bq. 2) I can't remember why the input stream is copied. but the
ByteArrayInputStream is not closed, as it will be freed automatically. I don't
think the config it used much elsewhere, so Ok not to close it.
Although not closing the ByteArrayInputStream does not have a negative impact
in fact the original input stream is closed. Keep that or change it?
bq. 3) ok. I don't think the config is used much elsewhere, so Ok to not close
it.
Are you proposing to change the behaviour?
> Clarify closing behaviour of input streams in methods getting it as argument
> ----------------------------------------------------------------------------
>
> Key: JCRVLT-349
> URL: https://issues.apache.org/jira/browse/JCRVLT-349
> Project: Jackrabbit FileVault
> Issue Type: Wish
> Components: vlt
> Affects Versions: 3.2.8
> Reporter: Konrad Windszus
> Assignee: Konrad Windszus
> Priority: Major
>
> There are several places in the FileVault code where an input stream is
> passed as argument to a method and closed within it (even though this is not
> explicitly mentioned in the javadocs). I think this is bad for the following
> reasons:
> # it is unexpected for the caller that the given input stream is closed (i.e.
> this is also never the case for methods taking an input stream in
> {{org.apache.commons.io.IOUtils}} nor in the JRE itself
> (https://docs.oracle.com/javase/7/docs/api/java/util/Properties.html#load(java.io.InputStream)))
> # closing the input stream might actually have an undesired effect if the
> input stream may contain additional input which is supposed to be consumed
> afterwards
> # closing often happens in a way that there may be leaks in case of
> exceptions.
> # the behaviour is not documented
> Examples for such behaviour are at
> #
> https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultMetaInf.java#L104
> #
> https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java#L364
> If I am not incorrect the behaviour is also not consistent as e.g. in
> https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultMetaInf.java#L136
> the input stream is not closed.
> Update: As it is too dangerous to change the semantics right now it should
> rather be consistently closed in all these methods in FileVault and clearly
> documented in the javadocs (although this would still deviate from the
> semantics of such methods in JRE and most other libraries).
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
