Does InputStream need to be closed if it's returned in a Response?

Question

I have piece of code similar to this:

@POST
@Produces(MediaType.APPLICATION_OCTET_STREAM)
@Path("test")
public Response getFileContent() {
    InputStream in = Files.newInputStream(filePath);
    return Response.ok(in,MediaType.APPLICATION_OCTET_STREAM);
}

Sonarcube complains that it isn't enclosed with try-with-resources. But when I do what it suggests, then I get java.nio.channels.ClosedChannelException when I try to call the endpoint.

Does InputStream really need to be manually closed by me if I'm returning it in a response? Doesn't the server takes care of that? And if it should be, then what is the recommended way to do it?

Answer 1

The way java is set up, it is impossible for a linting tool to reliably identify either side of this equation. Specifically:

• It is impossible to figure out given any method or constructor signature, that the caller is clearly the 'responsible' party for closing it. The closest you can get is the notion that calling a constructor of a type that implements AutoClosable implies it (true enough), but Files.newInputStream isn't one of those. You can't even go with: Either a constructor of an AutoClosable type, or a method that returns such a type and contains/starts with the letters n, e, w. After all, socket.getInputStream() is a resource that the caller should close, for example. In practice these linters ship with a known list, and therefore, the linter can and often is just wrong.

• It is also unknown if an endpoint takes over closing responsibilities. here, returning a Response object transfers the responsibility; the linter doesn't know it and there is no standard system to know it, other than building a giant list which this linter evidently did not do.

• It is even unknown if the resource represents a thing that ordinarily should be closed, but in this specific case should not be. Textbook example of this is new Scanner(System.in) - unless a linter has a hardcoded exception, they will tell you that you should close the scanner, and that advice would be dead wrong. You should in fact not close that scanner; closing it is broken code.

CONCLUSION: Linters are tools; simplifications that remind you to look at code. Blind application of what it tells you is silly and leads to crappy code. Don't do that. Tautologically then, any linting tool for which you cannot trivially tell the linter to shut up because it's wrong is a bad tool and you should immediately stop using it.

NB: as a pragmatic point, you can just return the Path object itself, i.e. return Response.ok(filePath, MediaType.APPLICATION_OCTET_STREAM), resteasy should know what to do with it. In the unlikely event it doesn't, filePath.toFile() will definitely work. However, that works here, but there are situations where all you have is an inputstream. Therefore, the principle applies.

To recap:

[1] Linters by their nature are often wrong. It is therefore required that the user of such tool understands that what they tell you is merely a hint; linter advice should under no circumstances be slavishly adhered to just because the linter said so.

[2] It is in fact impossible to write a linter that perfectly solves the problem of detecting use-without-close code errors.

[3] There is no problem here. Any linter tool being used must have an easy escape hatch, or it shouldn't be used.

Answer 2

Streaming should be closed by MessageBodyWriter implementation. If you want to be clear here - read your stream to bytes array and pass it as entity to response.

See here similar question