You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Funk <fu...@arcor.de> on 2007/09/25 22:05:59 UTC
Contract of Session.attach() and Session.detach()
Hi,
what is the contract of Session.attach() and Session.detach() ?
Especially, is it intended that after a call to attach() that there will
be at least one call to detach() before the request goes back to the client?
If that's the case, then there might be a bug in Session and I propose
the following patch on org.apache.wicket.Session
Index: .
===================================================================
--- . (revision 579354)
+++ . (working copy)
@@ -305,6 +305,11 @@
*/
public static void unset()
{
+ Session session = (Session)current.get();
+ if (session == null)
+ {
+ session.detach();
+ }
current.set(null);
}
Martin
Re: Contract of Session.attach() and Session.detach()
Posted by Martin Funk <fu...@arcor.de>.
Eelco Hillenius schrieb:
>> In my current project well fell over this looking at:
>>
>> WicketFilter.getLastModified(final HttpServletRequest servletRequest)
>> where cachable resources lead over Session.findOrCreate to Session.set(Session) to Session.attach()
>>
>> but the Session.unset() doesn't lead to a Session.detach()
>>
>
> Sounds wrong to me. Would you mind opening a JIRA issue for that please?
>
> Eelco
>
No I wouldn't mind:
https://issues.apache.org/jira/browse/WICKET-1010
Martin
Re: Contract of Session.attach() and Session.detach()
Posted by Maurice Marrink <ma...@gmail.com>.
If i recall correctly from way way back when wicket was still version
1.2.x :) there is no guarantee that session.detach will be called for
every session.attach due to some synchronized thingies :)
We are talking wicket 1.2 here so this might not be relevant in this
situation (any more) but i believe there were reasons not to
synchronize the complete request, performance being one of them.
Maurice
On 9/25/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > In my current project well fell over this looking at:
> >
> > WicketFilter.getLastModified(final HttpServletRequest servletRequest)
> > where cachable resources lead over Session.findOrCreate to Session.set(Session) to Session.attach()
> >
> > but the Session.unset() doesn't lead to a Session.detach()
>
> Sounds wrong to me. Would you mind opening a JIRA issue for that please?
>
> Eelco
>
Re: Contract of Session.attach() and Session.detach()
Posted by Eelco Hillenius <ee...@gmail.com>.
> In my current project well fell over this looking at:
>
> WicketFilter.getLastModified(final HttpServletRequest servletRequest)
> where cachable resources lead over Session.findOrCreate to Session.set(Session) to Session.attach()
>
> but the Session.unset() doesn't lead to a Session.detach()
Sounds wrong to me. Would you mind opening a JIRA issue for that please?
Eelco
Re: Contract of Session.attach() and Session.detach()
Posted by Martijn Dashorst <ma...@gmail.com>.
Note that the session.detach() should be performed in a try-finally
block, because the current.set(null) *HAS* to be performed regardless
of what happens, even if a nuke goes off on your server.
To add what Maurice said:
- Wicket session objects are not synchronized -> you can have race
conditions while accessing it
- don't assume that the session is not detached in your specific
thread because session.detach was not called in the thread
- for that matter, don't assume your session is attached during a
request either: right after your attach, another parallel running
request (e.g. for a resource!) can detach the session
- don't link your hibernate session to the wicket session but to the
request cycle
- don't store your hibernate entities in the wicket session, but keep
only their id's there. Make them request local.
Martijn
On 9/25/07, Martin Funk <fu...@arcor.de> wrote:
> darn, I thought you might fall for that :-)
>
> ok, so lets change it to:
>
> Index: .
> ===================================================================
> --- . (revision 579354)
> +++ . (working copy)
> @@ -305,6 +305,11 @@
> */
> public static void unset()
> {
> + Session session = (Session)current.get();
> + if (session != null)
> + {
> + session.detach();
> + }
> current.set(null);
> }
>
>
>
> In my current project well fell over this looking at:
>
> WicketFilter.getLastModified(final HttpServletRequest servletRequest)
> where cachable resources lead over Session.findOrCreate to Session.set(Session) to Session.attach()
>
> but the Session.unset() doesn't lead to a Session.detach()
>
> Martin
>
>
>
> Martijn Dashorst schrieb:
> > Why do you want a NullPointerException to occur in unset? :-)
> >
> > Martijn
> >
> > On 9/25/07, Martin Funk <fu...@arcor.de> wrote:
> >
> >> Hi,
> >>
> >> what is the contract of Session.attach() and Session.detach() ?
> >>
> >> Especially, is it intended that after a call to attach() that there will
> >> be at least one call to detach() before the request goes back to the client?
> >> If that's the case, then there might be a bug in Session and I propose
> >> the following patch on org.apache.wicket.Session
> >>
> >> Index: .
> >> ===================================================================
> >> --- . (revision 579354)
> >> +++ . (working copy)
> >> @@ -305,6 +305,11 @@
> >> */
> >> public static void unset()
> >> {
> >> + Session session = (Session)current.get();
> >> + if (session == null)
> >> + {
> >> + session.detach();
> >> + }
> >> current.set(null);
> >> }
> >>
> >>
> >> Martin
> >>
> >>
> >>
> >>
> >
> >
> >
>
>
--
Buy Wicket in Action: http://manning.com/dashorst
Apache Wicket 1.3.0-beta3 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta3/
Re: Contract of Session.attach() and Session.detach()
Posted by Martin Funk <fu...@arcor.de>.
darn, I thought you might fall for that :-)
ok, so lets change it to:
Index: .
===================================================================
--- . (revision 579354)
+++ . (working copy)
@@ -305,6 +305,11 @@
*/
public static void unset()
{
+ Session session = (Session)current.get();
+ if (session != null)
+ {
+ session.detach();
+ }
current.set(null);
}
In my current project well fell over this looking at:
WicketFilter.getLastModified(final HttpServletRequest servletRequest)
where cachable resources lead over Session.findOrCreate to Session.set(Session) to Session.attach()
but the Session.unset() doesn't lead to a Session.detach()
Martin
Martijn Dashorst schrieb:
> Why do you want a NullPointerException to occur in unset? :-)
>
> Martijn
>
> On 9/25/07, Martin Funk <fu...@arcor.de> wrote:
>
>> Hi,
>>
>> what is the contract of Session.attach() and Session.detach() ?
>>
>> Especially, is it intended that after a call to attach() that there will
>> be at least one call to detach() before the request goes back to the client?
>> If that's the case, then there might be a bug in Session and I propose
>> the following patch on org.apache.wicket.Session
>>
>> Index: .
>> ===================================================================
>> --- . (revision 579354)
>> +++ . (working copy)
>> @@ -305,6 +305,11 @@
>> */
>> public static void unset()
>> {
>> + Session session = (Session)current.get();
>> + if (session == null)
>> + {
>> + session.detach();
>> + }
>> current.set(null);
>> }
>>
>>
>> Martin
>>
>>
>>
>>
>
>
>
Re: Contract of Session.attach() and Session.detach()
Posted by Martijn Dashorst <ma...@gmail.com>.
Why do you want a NullPointerException to occur in unset? :-)
Martijn
On 9/25/07, Martin Funk <fu...@arcor.de> wrote:
> Hi,
>
> what is the contract of Session.attach() and Session.detach() ?
>
> Especially, is it intended that after a call to attach() that there will
> be at least one call to detach() before the request goes back to the client?
> If that's the case, then there might be a bug in Session and I propose
> the following patch on org.apache.wicket.Session
>
> Index: .
> ===================================================================
> --- . (revision 579354)
> +++ . (working copy)
> @@ -305,6 +305,11 @@
> */
> public static void unset()
> {
> + Session session = (Session)current.get();
> + if (session == null)
> + {
> + session.detach();
> + }
> current.set(null);
> }
>
>
> Martin
>
>
>
--
Buy Wicket in Action: http://manning.com/dashorst
Apache Wicket 1.3.0-beta3 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta3/