Use try-with-resources. #207
Conversation
|
conflicts with latest merge, please update the PR |
arturobernalg
force-pushed
the
feature/resource
branch
from
fb3244f to
4152a15
Compare
solved |
| @@ -108,8 +108,9 @@ public final int doStartTag() { | |||
| public final int doAfterBody() { | |||
| if( bodyContent != null ) { | |||
| try { | |||
| final JspWriter out = getPreviousOut(); | |||
| out.print(bodyContent.getString()); | |||
| try (JspWriter out = getPreviousOut()) { | |||
There was a problem hiding this comment.
as per JSPWriter#close, this method shouldn't be invoked directly.
Furthermore, JSPWriter#flush states [...] Once a stream has been closed, further write() or flush() invocations will cause an IOException to be thrown., so once this stream is closed, after the custom tags is rendered, the rest of the JSP wouldn't be rendered and will yield an exception? Would you mind verifying if this is the case?
There was a problem hiding this comment.
as per JSPWriter#close, this method shouldn't be invoked directly.
Furthermore, JSPWriter#flush states
[...] Once a stream has been closed, further write() or flush() invocations will cause an IOException to be thrown., so once this stream is closed, after the custom tags is rendered, the rest of the JSP wouldn't be rendered and will yield an exception? Would you mind verifying if this is the case?
Hey @juanpablo-santos I'll take a look. I'll let you know. In the meantime, what do you think about leaving the PR open?
TY
There was a problem hiding this comment.
You are absolutely right. After reviewing the JSPWriter documentation and the code again, I agree that closing the JSPWriter stream within the doAfterBody() method could potentially cause an IOException in the subsequent operations by the JSP engine.
I had introduced the try-with-resources to ensure proper resource handling. However, in this context, the JSP engine manages the lifecycle of the JSPWriter stream, and it is best not to interfere with this management.
Therefore, I will revert the try-with-resources change for the JspWriter out stream. Thank you for catching this and for your valuable feedback.
arturobernalg
force-pushed
the
feature/resource
branch
2 times, most recently
from
031b4ef to
bf6ebbf
Compare
|
Hi! just've seen now a pending review here - there're still a bunch of |
arturobernalg
force-pushed
the
feature/resource
branch
from
bf6ebbf to
266a049
Compare
No description provided.