[
https://issues.apache.org/jira/browse/TAP5-1355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935740#action_12935740
]
Andy Blower commented on TAP5-1355:
-----------------------------------
May I ask why my original patch to fix the issue TAP5-834 is not being
considered. I was pretty surprised that it was not used in the first place, but
the second way of fixing the issue was chosen which had possible concurrency
issues. My original words:
"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."
The second solution was used, but the first solution (which my patch
implemented) is much better. I know it adds a new method to a public
interface, but this is not as much impact as changing or removing an existing
method one and in this case is trivial for users to implement. There has
certainly been worse backwards compatibility changes made in T5, and the
benefit of this simple solution (#1 above) far outweighs the impact IMHO. It's
also a lot simpler and cleaner than the solutions proposed here. Also IMHO.
Just FYI the code from the patch was run in our production environment for
several months before we upgraded to Tapestry 5.2 and removed my
customizations. Luckily we don't seem to have run into any threading issues,
well not that have caused any fuss at least.
> Threading issue with SessionStateObjects
> ----------------------------------------
>
> Key: TAP5-1355
> URL: https://issues.apache.org/jira/browse/TAP5-1355
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.2.4
> Reporter: Moritz Gmelin
> Attachments: Screenshot.png.jpg, taptest.tgz
>
>
> When a page request consists of multiple HTTP request (e.g. page and some
> dynamically generated images) and all those requests access a
> SessionStateObject, it happens that a new session (with an empty SSO) is
> created for some of the request threads.
> I was able to create a very simple example to recreate that problem:
> -A simple page that displays 20 dynamically generated images in a loop.
> -In the page, a SSO, holding a number value is initialized to a random
> number.
> -Each of the dynamic images read that number and draws it.
> -Make sure that a HTTP-Request is made for every image on the page (by
> adding some random number to the event link)
> The effect that you'll see after some reloads of the page (maybe you need to
> reload 30 times) is that some images will draw 0 as SSO value instead of the
> number set in the page @BeginRender method. Those fields will be marked in
> red in the demo so you can quickly see them.
> I definitely beleive that tapestry should take care of this. It is a use case
> for SSOs that is probably too common to ignore.
> Why can't this be automatically integrated into the ApplicationStateManager?
>
> The demo has been deployed here
> http://www.avetana.de/taptest/
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
