Fwd: Usage of file handles/file streams in GWT source code

104 views
Skip to first unread message

Thomas Broyer

unread,
Jun 12, 2017, 5:29:15 AM6/12/17
to Google Web Toolkit Contributors
Moving discussion to GWT Contrib.

On Sunday, June 11, 2017 at 9:35:30 PM UTC+2, mr.g...@gmail.com wrote:
Reading and writing of files should only be done using try-with-resources. This might not be an issue on Linux, but it is a serious issue on Windows where I repeatedly get conflicts with GWT DevMode server not releasing file handles, ie. resulting in situations where it cannot even read its own cache files.

One example: Output.openSourceFile returns a file input stream. SourceHandler.makeSourcePage then does

return Responses.newBinaryStreamResponse("text/plain", pageBytes);

where it eventually might get closed if no error happens... and on top of that the stream is kept open as long as the client wishes to stall reading the response data.

I suggest setting up a GWT-internal standard for handling file resources. My recommendation would be to use try-with-resources and processing a file copy that is stored fully in RAM.


Colin Alworth

unread,
Jun 12, 2017, 10:57:28 AM6/12/17
to google-web-tool...@googlegroups.com
I fixed a few of these once upon a time in 2.6 or so to deal with an fd exhaustion issue on linux, would be willing to take on more. In that case even in success they were leaking, as opposed to only causing issues in cases of failures (more on this below).

If you have a list of cases that are used wrong (returning an InputStream isn't necessarily bad of course, since the calling code itself might have a try-with-resources), I can hit them and see what falls out.

I'm not sure copying literally every InputStream you see into a ByteArrayInputStream is a constructive suggestion, since of course to be defensive, anything given an InputStream needs to make yet another copy, or all of these APIs need to be adjusted to take ByteArrayInputStream, etc as a parameter to be clear that the stream has been closed. Is there another best practice that can make this cleaner, perhaps rewritting something like the Response interface so it implements AutoClosable so that _it_ can be try-with-resource'd?

Quickly reviewing all cases where Responses.newBinaryStreamResponse is called (transitively), all usages either return right away (so no error can happen), or com.google.gwt.dev.codeserver.WebServer#handleRequest where yes, it is possible for a NPE or something could happen prior to page-send (which then cleans up correctly in a try/finally), but that would point to rather broken code. Response can't be null, target can't be null, the logger really shouldn't throw and the response shouldn't be able to throw in setHeader). Or is there something concerning in handleRequest that you see which can surprisingly error out before page.send(...) can be reached?

--
  Colin Alworth

--
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-co...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply all
Reply to author
Forward
0 new messages