You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by "Andy Blower (JIRA)" <ji...@apache.org> on 2009/09/03 15:56:57 UTC
[jira] Created: (TAP5-834) BaseOptimizedSessionPersistedObject does
not work correctly with Tomcat & Jetty
BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
-------------------------------------------------------------------------------
Key: TAP5-834
URL: https://issues.apache.org/jira/browse/TAP5-834
Project: Tapestry 5
Issue Type: Bug
Components: tapestry-core
Affects Versions: 5.0.18, 5.1.0.5, 5.1.0.4, 5.1.0.3, 5.1.0.2, 5.1.0.1, 5.1.0.0
Reporter: Andy Blower
Priority: Critical
OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
>From Tomcat StandardSession.java:
// Call the valueBound() method if necessary
if (notify && value instanceof HttpSessionBindingListener) {
// Don't call any notification if replacing with the same value
Object oldValue = attributes.get(name);
if (value != oldValue) {
event = new HttpSessionBindingEvent(getSession(), name, value);
try {
((HttpSessionBindingListener) value).valueBound(event);
} catch (Throwable t){
manager.getContainer().getLogger().error
(sm.getString("standardSession.bindingEvent"), t);
}
}
}
So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Howard M. Lewis Ship reassigned TAP5-834:
-----------------------------------------
Assignee: Howard M. Lewis Ship
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Andy Blower (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12788043#action_12788043 ]
Andy Blower commented on TAP5-834:
----------------------------------
It looks to me like you've implemented solution 2 from my bug report above. As I said originally, I'm concerned that this will potentially cause hard to find concurrency problems which is why I thought the more complicated solution was a better option. If there are two threads handling requests for the same user session, setting the session attribute to null without synchronisation when one request has finished processing while the other is still being handled could cause problems, couldn't it?
If you think that this isn't an issue, please could you explain why? Maybe I'm missing something in my analysis here?
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Andy Blower (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12788796#action_12788796 ]
Andy Blower commented on TAP5-834:
----------------------------------
It is definitely an edge case, and maybe I'm being over paranoid. As you say solution #1 (implemented in my patch) does change two public interfaces. I don't consider it an issue because it's only adding a single new a method in each case, which for people upgrading to T5.2 would be as simple as generating/writing a method stub that do nothing to keep the same behaviour as T5.1. I could see your point if it changed existing method signatures or something, but I think there were much worse changes required for migrating from 5.0->5.1, and I feel that this is better for T5 than adding synchronization overheads. That's my opinion anyway FWIW. :-)
Thanks for the response. Never heard of Brian Goetz before though...
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Andy Blower (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12788043#action_12788043 ]
Andy Blower commented on TAP5-834:
----------------------------------
It looks to me like you've implemented solution 2 from my bug report above. As I said originally, I'm concerned that this will potentially cause hard to find concurrency problems which is why I thought the more complicated solution was a better option. If there are two threads handling requests for the same user session, setting the session attribute to null without synchronisation when one request has finished processing while the other is still being handled could cause problems, couldn't it?
If you think that this isn't an issue, please could you explain why? Maybe I'm missing something in my analysis here?
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Andy Blower (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12788796#action_12788796 ]
Andy Blower commented on TAP5-834:
----------------------------------
It is definitely an edge case, and maybe I'm being over paranoid. As you say solution #1 (implemented in my patch) does change two public interfaces. I don't consider it an issue because it's only adding a single new a method in each case, which for people upgrading to T5.2 would be as simple as generating/writing a method stub that do nothing to keep the same behaviour as T5.1. I could see your point if it changed existing method signatures or something, but I think there were much worse changes required for migrating from 5.0->5.1, and I feel that this is better for T5 than adding synchronization overheads. That's my opinion anyway FWIW. :-)
Thanks for the response. Never heard of Brian Goetz before though...
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (TAP5-834) BaseOptimizedSessionPersistedObject does
not work correctly with Tomcat & Jetty
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Howard M. Lewis Ship closed TAP5-834.
-------------------------------------
Resolution: Fixed
Fix Version/s: 5.2.0.0
Chose a simpler, alternate solution, that when the attribute is dirty, we set the attribute to null, then back to the dirty object. This should force the servlet container to trigger the notifications, and it doesn't involve a change to (what I expect to be) a somewhat common interface that much end-user code will implement.
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (TAP5-834) BaseOptimizedSessionPersistedObject does
not work correctly with Tomcat & Jetty
Posted by "Andy Blower (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Andy Blower updated TAP5-834:
-----------------------------
Attachment: TAP5-834-patch.txt
As there doesn't appear to be anyone else who's bothered about clustering performance with Tapestry 5, we've implemented solution 1and tested it with our application. I've attached the patch which I hope will make its way into the Tapestry svn repository and thus into next Tapestry 5 release.
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Priority: Critical
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (TAP5-834) BaseOptimizedSessionPersistedObject does
not work correctly with Tomcat & Jetty
Posted by "Andy Blower (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Andy Blower updated TAP5-834:
-----------------------------
Attachment: TAP5-834-patch.txt
As there doesn't appear to be anyone else who's bothered about clustering performance with Tapestry 5, we've implemented solution 1and tested it with our application. I've attached the patch which I hope will make its way into the Tapestry svn repository and thus into next Tapestry 5 release.
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Priority: Critical
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12788784#action_12788784 ]
Howard M. Lewis Ship commented on TAP5-834:
-------------------------------------------
It does conform to Brian Goetz's theory that all web applications are broken.
My analysis is that event requests are the ones likely to modify server-side state and they redirect to page render requests and asset requests, which do not modify state generally (though the flash persistence type does).
Even assuming sticky sessions, there's a question about how to manage this concurrency best.
It may be necessary to but a mutex into the ComponentEventRequestHandler pipeline when there's an active session for the request.
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (TAP5-834) BaseOptimizedSessionPersistedObject does
not work correctly with Tomcat & Jetty
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Howard M. Lewis Ship closed TAP5-834.
-------------------------------------
Resolution: Fixed
Fix Version/s: 5.2.0.0
Chose a simpler, alternate solution, that when the attribute is dirty, we set the attribute to null, then back to the dirty object. This should force the servlet container to trigger the notifications, and it doesn't involve a change to (what I expect to be) a somewhat common interface that much end-user code will implement.
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Howard M. Lewis Ship reassigned TAP5-834:
-----------------------------------------
Assignee: Howard M. Lewis Ship
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TAP5-834) BaseOptimizedSessionPersistedObject
does not work correctly with Tomcat & Jetty
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12788784#action_12788784 ]
Howard M. Lewis Ship commented on TAP5-834:
-------------------------------------------
It does conform to Brian Goetz's theory that all web applications are broken.
My analysis is that event requests are the ones likely to modify server-side state and they redirect to page render requests and asset requests, which do not modify state generally (though the flash persistence type does).
Even assuming sticky sessions, there's a question about how to manage this concurrency best.
It may be necessary to but a mutex into the ComponentEventRequestHandler pipeline when there's an active session for the request.
> BaseOptimizedSessionPersistedObject does not work correctly with Tomcat & Jetty
> -------------------------------------------------------------------------------
>
> Key: TAP5-834
> URL: https://issues.apache.org/jira/browse/TAP5-834
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.0.18
> Reporter: Andy Blower
> Assignee: Howard M. Lewis Ship
> Priority: Critical
> Fix For: 5.2.0
>
> Attachments: TAP5-834-patch.txt
>
>
> OptimizedSessionPersistedObject's suggestion for implementing isSessionPersistedObjectDirty(), as used by BaseOptimizedSessionPersistedObject, does not work correctly with Tomcat & Jetty. (and quite possibly other servlet containers too, but we only use Jetty & Tomcat so have only confirmed it with them)
> OptimizedSessionPersistedObject model relies on the servlet container session object triggering a HttpSessionBindingEvent when an object is re-stored in the session to reset the dirty flag. I've only looked at the source of Tomcat 5.5 and 6 but when an object is replaced in the session using setAttribute() the new object and the existing object are compared by reference only, if they both refer to the same object then no HttpSessionBindingEvent is triggered.
> From Tomcat StandardSession.java:
> // Call the valueBound() method if necessary
> if (notify && value instanceof HttpSessionBindingListener) {
> // Don't call any notification if replacing with the same value
> Object oldValue = attributes.get(name);
> if (value != oldValue) {
> event = new HttpSessionBindingEvent(getSession(), name, value);
> try {
> ((HttpSessionBindingListener) value).valueBound(event);
> } catch (Throwable t){
> manager.getContainer().getLogger().error
> (sm.getString("standardSession.bindingEvent"), t);
> }
> }
> }
> So, using OptimizedSessionPersistedObject, there is currently no way of setting the dirty flag to false after the object has been saved in the session - hence we are propagating all of the SSOs across the cluster all of the time because the dirty flag stays set to true.
> I think there are two possible solutions to this issue - I prefer the first by a large margin, but both modify the SessionImpl.restoreDirtyObjects() method.
> 1) Add a new method to OptimizedSessionPersistedObject interface to reset the dirty flag, and a corresponding method in SessionPersistedObjectAnalyzer - implementing them as appropriate, then call the new reset method after setting the session attribute in SessionImpl.restoreDirtyObjects().
> 2) Remove the session attribute before adding it in SessionImpl.restoreDirtyObjects(). Although I have a worry that this may potentially cause hard to find concurrency problems.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.