readWithMessageConverters opens a FileInputStream but does not ensure it closes after reading from it. Ensure the FileInputStream is closed.
Solves Issue: https://github.com/spring-projects/spring-framework/issues/25896
Comment From: pivotal-issuemaster
@SenseiFisher Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.
Comment From: pivotal-issuemaster
@SenseiFisher Thank you for signing the Contributor License Agreement!
Comment From: rstoyanchev
I don't think it should be closed as it may not be the end of the response, for example in a multipart request but it could also be in another streaming scenario. See for example #14728 or the more recent regression #25989 caused by a similar fix.
Is there a specific issue that this presents?
Comment From: SenseiFisher
Hi, I recived a "Too many opened files" (50,000) a few times at the production environment. After investigating it seems like the files belongs to tomcat.
I ran some tests at our development environment and saw the files only closed at GC when using the StringHttpConverter (finalize).
This led me to belive it just a matter of luck, if GCs are not executed frequent enough, every system will recive this exact same error.
בתאריך יום ב׳, 2 בנוב׳ 2020, 15:44, מאת Rossen Stoyanchev notifications@github.com:
I don't think it should be closed as it may not be the end of the response, for example in a multipart request but it could also be in another streaming scenario. See for example #14728 https://github.com/spring-projects/spring-framework/issues/14728 or the more recent regression #25989 https://github.com/spring-projects/spring-framework/issues/25989 caused by a similar fix.
Is there a specific issue that this presents?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-framework/pull/25897#issuecomment-720480465, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKKQXHJYOKJ5B24MWTQSZTLSN2ZTDANCNFSM4SMVNCLQ .
Comment From: jhoeller
Since this is on the input side, we could potentially close it once the request body has been fully read. I'm less concerned there than on the output side but still wondering why it would be necessary.
Where does the FileInputStream come from, actually? Is it Tomcat opening such a stream?
Comment From: SenseiFisher
Well... The InputSteam is created on the EmptyBodyCheckingHttpInputMessage constructor, and the instance is stored in "message".
After some processing, the function ends and return the processed body. Making the "message" unreachable.
As a result once the function ends the InputStream cannot be closed manually.
בתאריך יום ג׳, 1 בדצמ׳ 2020, 10:12, מאת Juergen Hoeller notifications@github.com:
Since this is on the input side, we could potentially close it once the request body has been fully read. I'm less concerned there than on the output side but still wondering why it would be necessary.
Where does the FileInputStream come from, actually? Is it Tomcat opening such a stream?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-framework/pull/25897#issuecomment-736300545, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKKQXHJCJB3QLWYDTNIB5ITSSSQPRANCNFSM4SMVNCLQ .
Comment From: rstoyanchev
Scheduling for consideration in 6.0 M2 along with #27773.
Where does the FileInputStream come from, actually? Is it Tomcat opening such a stream?
If it is related to reading parts, as in #27773, then the the underlying input stream could be a temp file for the part.
Comment From: rstoyanchev
We're going to address this in a similar way but with a slightly different implementation, so I'm closing this as superseded by #27773.