You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Willis Blackburn (JIRA)" <ji...@apache.org> on 2011/01/02 22:30:46 UTC
[jira] Created: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Endless recursion if LoadableDetachableModel.load throws exception
------------------------------------------------------------------
Key: WICKET-3302
URL: https://issues.apache.org/jira/browse/WICKET-3302
Project: Wicket
Issue Type: Bug
Components: wicket
Affects Versions: 1.5-M3
Reporter: Willis Blackburn
I don't know if there's any easy fix for this, but here's what happens:
1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
2. The model is a LoadableDetachableModel.
3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
4. This gets caught in Component.getDefaultModelObject:
log.error("Error while getting default model object for Component: " + this.toString(true));
5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Willis Blackburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976564#action_12976564 ]
Willis Blackburn commented on WICKET-3302:
------------------------------------------
Maybe a good solution would be to have Component.toString catch any exceptions that occur during the rendering of the string and fall back on emitting just the component ID plus "caught TheBigException while retrieving component details."
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Attila Király (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977294#action_12977294 ]
Attila Király commented on WICKET-3302:
---------------------------------------
As I see there was a change in wicket in 1.4.10 (see [1]) so the new prefered way is to override onConfigure() (which is called only once) and set visibility from there instead of overriding isVisible() (which is called several times). Guess if the visibility controlling logic is in onConfigure() there is no endless recursion.
[1]: http://wicket.apache.org/2010/08/11/wicket-1.4.10-released.html
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Willis Blackburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977612#action_12977612 ]
Willis Blackburn commented on WICKET-3302:
------------------------------------------
Igor--thanks for addressing this. onConfigure does sound like a good solution, and I'll start using it (wasn't aware of it until now!) but unless isVisible (and any other Component method invoked during exception processing) becomes final, there's always the chance of someone providing an implementation that uses the uninitialized model.
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
> Assignee: Igor Vaynberg
> Fix For: 1.4.16, 1.5-M4
>
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg reassigned WICKET-3302:
-------------------------------------
Assignee: Igor Vaynberg
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
> Assignee: Igor Vaynberg
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Willis Blackburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977268#action_12977268 ]
Willis Blackburn commented on WICKET-3302:
------------------------------------------
I know that it's supposed to work that way because I read about people having this problem before. I did some more investigation and discovered that the exception from LDM.load is only the instigator. The cause of the recursion is that after the first invocation of the Component's isVisible method, which calls LDM.load and generates the exception, subsequent calls generate NPE, because the LDM.getObject now returns null. The recursion continues until either we blow the stack or run out of heap. Since this happens in less than a second normally, it sort of looks like the exception has worked and has aborted the page render, but the requested status code never gets sent back to the browser.
I think that catching exceptions in Component.toString would be a good idea and would resolve this issue and many future issues. Throwing an exception from the getDefaultModelObject method for any reason (whether it's due to LDM.load or from some custom model code) leaves the component's model in an unknown state, which means that any call to any non-final method of Component might generate an exception, which in turn will lead to the stack and/or heap being blown.
I guess you could say "well it's the programmer's responsibility to check for a null model," but this seems unreasonable in light of the circumstances:
1. The programmer is throwing an exception from LDM.load specifically because he doesn't want to deal with having a null model. If the code was prepared to deal with a null model, then the programmer could just return null from LDM.load and let the page render and/or handle the condition in some other manner.
2. If the programmer has thrown AbortWithHttpErrorCodeException, then he reasonably expects the request to *abort*, not to call additional component methods. There is no compelling reason why Wicket can't just set the HTTP status and commit the response.
3. The method that is leading to the recursion is a *diagnostic* method (toString), not one that's part of Wicket's core logic. The programmer should not have to add "if (model != null) ..." throughout his code merely to support a log message. If Wicket wants to print some diagnostics about a component with a known-bad model, then it should handle the potential exceptions. The programmer, who has tried to avoid working with a bad model by thrown an exception from LDM.load, should not have to write defensive code anyway to support Component, which is choosing to continue to invoke methods on itself despite the model having failed to initialize.
W
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Pedro Santos (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977249#action_12977249 ]
Pedro Santos commented on WICKET-3302:
--------------------------------------
Hi Willis, even if you throw an exception at the LDM#load method, it has the 'attached' flag that will prevent an second load try, preventing your described issue. Where did you throw the AbortWithHttpErrorCodeException? Can you provide an test case?
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg resolved WICKET-3302.
-----------------------------------
Resolution: Fixed
Fix Version/s: 1.5-M4
1.4.16
Attila is correct, onConfigure() is a better place to do this. However, I added the guard anyways to be on the safe side.
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
> Assignee: Igor Vaynberg
> Fix For: 1.4.16, 1.5-M4
>
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977355#action_12977355 ]
Hudson commented on WICKET-3302:
--------------------------------
Integrated in Apache Wicket 1.4.x #367 (See [https://hudson.apache.org/hudson/job/Apache%20Wicket%201.4.x/367/])
Issue: WICKET-3302
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
> Assignee: Igor Vaynberg
> Fix For: 1.4.16, 1.5-M4
>
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Willis Blackburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977611#action_12977611 ]
Willis Blackburn commented on WICKET-3302:
------------------------------------------
Pedro--the NPE is in my component's isVisible method. In a roundabout way the isVisible method eventually invokes a method on the object returned from LDM.getObject.
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
> Assignee: Igor Vaynberg
> Fix For: 1.4.16, 1.5-M4
>
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3302) Endless recursion if
LoadableDetachableModel.load throws exception
Posted by "Pedro Santos (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977358#action_12977358 ]
Pedro Santos commented on WICKET-3302:
--------------------------------------
@Willis: "subsequent calls generate NPE" where? can you copy past the relevant stack trace?
> Endless recursion if LoadableDetachableModel.load throws exception
> ------------------------------------------------------------------
>
> Key: WICKET-3302
> URL: https://issues.apache.org/jira/browse/WICKET-3302
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Willis Blackburn
> Assignee: Igor Vaynberg
> Fix For: 1.4.16, 1.5-M4
>
>
> I don't know if there's any easy fix for this, but here's what happens:
> 1. Component subclass overrides isVisible with a new implementation that depends on the current model state.
> 2. The model is a LoadableDetachableModel.
> 3. On the initial render, the load method of the LoadableDetachableModel throws a RuntimeException for whatever reason. (In my case I was trying to throw a AbortWithHttpErrorCodeException.)
> 4. This gets caught in Component.getDefaultModelObject:
> log.error("Error while getting default model object for Component: " + this.toString(true));
> 5. The toString method that's invoked from the exception handler prints the component's state, including its visibility.
> 6. In order to resolve the visibility state, toString has to call isVisible--the same method that initially caused the exception.
> 7. The isVisible method again throws an exception, etc.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.