Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear up InputStream ownership #8

Open
hakanai opened this issue Mar 19, 2017 · 0 comments
Open

Clear up InputStream ownership #8

hakanai opened this issue Mar 19, 2017 · 0 comments

Comments

@hakanai
Copy link
Contributor

hakanai commented Mar 19, 2017

I found that the COPY method wasn't closing file streams.

The servlet calls this to get the source data:

    InputStream getResourceContent(ITransaction transaction, String resourceUri);

Then it does the copy by passing it to:

    long setResourceContent(ITransaction transaction, String resourceUri,
            InputStream content, String contentType, String characterEncoding);

But after doing the copy, it doesn't close the stream.

On further investigation, I found that LocalFileSystemStore was handling this by closing the stream inside its implementation of setResourceContent. This seems odd to me, because the stream is passed from the outside, which implies that the caller owns it, and thus the caller should surely be responsible for closing it.

Maybe this could be cleared up, either by flipping the responsibilities around to close it from the caller, or by clearly documenting on setResourceContent the contract that the implementation is expected to close the stream. (The caveat of the latter being that if there was an issue calling the method, the code which closes the stream might not be called, i.e., the only really safe place to have the closing code is at the caller.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant