You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Scott Oaks (JIRA)" <de...@myfaces.apache.org> on 2011/06/10 17:56:58 UTC
[jira] [Created] (PORTLETBRIDGE-214) BridgeImpl incorrectly cleans
up after exceptions; retains contexts
BridgeImpl incorrectly cleans up after exceptions; retains contexts
-------------------------------------------------------------------
Key: PORTLETBRIDGE-214
URL: https://issues.apache.org/jira/browse/PORTLETBRIDGE-214
Project: MyFaces Portlet Bridge
Issue Type: Bug
Components: Impl
Affects Versions: 2.0.0
Reporter: Scott Oaks
Assignee: Michael Freedman
The exception handling of BridgeImpl.doFacesRequest() is incorrect, which leads to contexts not being released after an exception.
When an exception is thrown to the doFacesRequest() method, it ends up in this code:
try {
...
} catch (Exception e) {
...
context.getExternalContext().log("Exception thrown in doFacesRequest:resource", e); // line 1168
...
} finally {
...
context.release();
...
}
The first problem is that whatever error is getting thrown to us is lost because line 1168 is generated a NullPointerException from context.getExternalContext().log(). So that NPE gets thrown from the exception block and the original, actual root-cause, exception is lost.
The reason that this code fails is that context.getExternalContext() returns null -- the processing has been redirected, and this context has already been released in redirectRender(). Which leads to the much more serious issue -- the new context established by redirectRender() is never released in the exception handling: the context.release() call in the finally block of doFacesRequest() is the original, already released context.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PORTLETBRIDGE-214) BridgeImpl incorrectly
cleans up after exceptions; retains contexts
Posted by "Scott Oaks (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/PORTLETBRIDGE-214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13047398#comment-13047398 ]
Scott Oaks commented on PORTLETBRIDGE-214:
------------------------------------------
Unfortunately, the testbed in question is production-mode; it is a little hard to get access. I will see what we can do -- if there is something we can catch after the fact it works better (e.g. I expect that lots of things call FacesContext.release() so we can't just generally try and trace that; the bug may happen once a day on a moderately-used server). If we knew the underlying exception that triggered the bug, it would likely help, so maybe as a first step we can just fix the NPE issue in the exception handler and see what information that gets us as to what triggers the release.
I do see that the doFacesRequest(RenderRequest request, RenderResponse response) reacquires the context in the exception clause (where doFacesRequest(ResourceRequest... doesn't). So I guess you are saying that the premature release in this case didn't come from the renderRedirect() method as I thought it must have, because that would have been a different entry point? That is certainly possible; I should have made clear that I was just looking for possible explanations at that point, and that seemed a likely one to me, though I don't have an understanding of what is actually going on.
> BridgeImpl incorrectly cleans up after exceptions; retains contexts
> -------------------------------------------------------------------
>
> Key: PORTLETBRIDGE-214
> URL: https://issues.apache.org/jira/browse/PORTLETBRIDGE-214
> Project: MyFaces Portlet Bridge
> Issue Type: Bug
> Components: Impl
> Affects Versions: 2.0.0
> Reporter: Scott Oaks
> Assignee: Michael Freedman
> Attachments: stack.txt
>
>
> The exception handling of BridgeImpl.doFacesRequest() is incorrect, which leads to contexts not being released after an exception.
> When an exception is thrown to the doFacesRequest() method, it ends up in this code:
> try {
> ...
> } catch (Exception e) {
> ...
> context.getExternalContext().log("Exception thrown in doFacesRequest:resource", e); // line 1168
> ...
> } finally {
> ...
> context.release();
> ...
> }
> The first problem is that whatever error is getting thrown to us is lost because line 1168 is generated a NullPointerException from context.getExternalContext().log(). So that NPE gets thrown from the exception block and the original, actual root-cause, exception is lost.
> The reason that this code fails is that context.getExternalContext() returns null -- the processing has been redirected, and this context has already been released in redirectRender(). Which leads to the much more serious issue -- the new context established by redirectRender() is never released in the exception handling: the context.release() call in the finally block of doFacesRequest() is the original, already released context.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PORTLETBRIDGE-214) BridgeImpl incorrectly
cleans up after exceptions; retains contexts
Posted by "Michael Freedman (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/PORTLETBRIDGE-214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13047344#comment-13047344 ]
Michael Freedman commented on PORTLETBRIDGE-214:
------------------------------------------------
Okay -- looked at the code. I understand what you have said but don't understand how we got in this situation as a redirect during a resource request is a noop (in a portlet/bridge environment). Is there any chance you can set a breakpoint in the bridge's FacesContext.release() and send me the stack so I can understand/work backwards in terms of why the context has been prematurely released ... Note: if you look at the corresponding doFacesRequest code for render requests -- you will see it does reacquire the FacesContext in the exception clause as it does expect a render redirect to occur.
I.e. the issue here seems to be an "unexpected" premature release of the facesContext in a resource request. Can you help me dig/determine why this is happening?
> BridgeImpl incorrectly cleans up after exceptions; retains contexts
> -------------------------------------------------------------------
>
> Key: PORTLETBRIDGE-214
> URL: https://issues.apache.org/jira/browse/PORTLETBRIDGE-214
> Project: MyFaces Portlet Bridge
> Issue Type: Bug
> Components: Impl
> Affects Versions: 2.0.0
> Reporter: Scott Oaks
> Assignee: Michael Freedman
> Attachments: stack.txt
>
>
> The exception handling of BridgeImpl.doFacesRequest() is incorrect, which leads to contexts not being released after an exception.
> When an exception is thrown to the doFacesRequest() method, it ends up in this code:
> try {
> ...
> } catch (Exception e) {
> ...
> context.getExternalContext().log("Exception thrown in doFacesRequest:resource", e); // line 1168
> ...
> } finally {
> ...
> context.release();
> ...
> }
> The first problem is that whatever error is getting thrown to us is lost because line 1168 is generated a NullPointerException from context.getExternalContext().log(). So that NPE gets thrown from the exception block and the original, actual root-cause, exception is lost.
> The reason that this code fails is that context.getExternalContext() returns null -- the processing has been redirected, and this context has already been released in redirectRender(). Which leads to the much more serious issue -- the new context established by redirectRender() is never released in the exception handling: the context.release() call in the finally block of doFacesRequest() is the original, already released context.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (PORTLETBRIDGE-214) BridgeImpl incorrectly cleans
up after exceptions; retains contexts
Posted by "Michael Freedman (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/PORTLETBRIDGE-214?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Freedman resolved PORTLETBRIDGE-214.
--------------------------------------------
Resolution: Fixed
Fix Version/s: 2.0.1
The NPE is likley caused because of failure during the acquisition of the FacesContext -- hence its not established/remains null. Changed code to use PortletConfig to log which is always around. In addition I noticed this and some other conditional logging checks were logging only if logging was disabled. Switched this to ensure they get logged only if logging is enabled.
> BridgeImpl incorrectly cleans up after exceptions; retains contexts
> -------------------------------------------------------------------
>
> Key: PORTLETBRIDGE-214
> URL: https://issues.apache.org/jira/browse/PORTLETBRIDGE-214
> Project: MyFaces Portlet Bridge
> Issue Type: Bug
> Components: Impl
> Affects Versions: 2.0.0
> Reporter: Scott Oaks
> Assignee: Michael Freedman
> Fix For: 2.0.1
>
> Attachments: stack.txt
>
>
> The exception handling of BridgeImpl.doFacesRequest() is incorrect, which leads to contexts not being released after an exception.
> When an exception is thrown to the doFacesRequest() method, it ends up in this code:
> try {
> ...
> } catch (Exception e) {
> ...
> context.getExternalContext().log("Exception thrown in doFacesRequest:resource", e); // line 1168
> ...
> } finally {
> ...
> context.release();
> ...
> }
> The first problem is that whatever error is getting thrown to us is lost because line 1168 is generated a NullPointerException from context.getExternalContext().log(). So that NPE gets thrown from the exception block and the original, actual root-cause, exception is lost.
> The reason that this code fails is that context.getExternalContext() returns null -- the processing has been redirected, and this context has already been released in redirectRender(). Which leads to the much more serious issue -- the new context established by redirectRender() is never released in the exception handling: the context.release() call in the finally block of doFacesRequest() is the original, already released context.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira