Can't we just make a different PageRequestTarget implementation (or constructor)
that takes a Class argument and then in respond we really make the page
and the respond is synched by default so then everything works fine.
Just delay the creation a bit. I think this doesn't matter for bookmarkable pages..
Except..... that checkaccess is done now before page.doRender() and (not in the respond anymore..)
It is one of the steps....
On the other hand.. Check Access should i think also be synched! Because most of the time you
want a session object (a user) so that is also not thread safe.
johan
On 1/5/06,
Jonathan Locke <[EMAIL PROTECTED]> wrote:
in fact this is way better because the lock granularity is flexible and
in the hands of the target implementation!
in the bookmarkable page case, the bookmarkable page is locked, but not
when it's a redirect:
/**
* @see wicket.IRequestTarget#getLock(RequestCycle)
*/
public Object getLock(RequestCycle requestCycle)
{
// we need to lock when we are not redirecting, i.e. we are
// actually rendering the page
return !requestCycle.getRedirect() ? requestCycle.getSession() :
null;
}
one can imagine other locking techniques as the result of getLock()
implementations...
Jonathan Locke wrote:
>
> no, no, you're right. that call to get the request target from the
> request cycle should just be above the factory call and the lock
> should work.
> sorry, i was jumping to conclusions...
>
> Igor Vaynberg wrote:
>> currently we lock the processing on the target.getLock()....why not
>> lock the page instantiation on the same lock?
>>
>> -Igor
>>
>>
>> On 1/5/06, *Jonathan Locke * <[EMAIL PROTECTED]
>> <mailto:[EMAIL PROTECTED]>> wrote:
>>
>>
>> also, because we have Session.get(), any non-locked object (image
>> object, for example)
>> that accesses the session this way could have threading
>> problems. but
>> that doesn't
>> seem very likely.
>>
>> Jonathan Locke wrote:
>> >
>> > just realized that the target lock problem will get much worse
>> if igor
>> > ever gets his way with delayed init. then components could
>> naively
>> > do something involving the parent that wound up accessing the
>> session.
>> >
>> > Jonathan Locke wrote:
>> >>
>> >> i think you should be synchronizing on the target's lock,
>> retrieved
>> >> by target.getLock()
>> >>
>> >> Martijn Dashorst wrote:
>> >>> Here's my patch for this code. I think this should work but
>> I'd like
>> >>> others to take a look at it:
>> >>>
>> >>> I changed the page construction, such that it is done within a
>> >>> synchronized block which synchronizes on the session object.
>> >>>
>> >>> In class:
>> wicket.request.compound.DefaultRequestTargetResolverStrategy
>> >>>
>> >>> protected IRequestTarget
>> resolveBookmarkablePage(RequestCycle
>> >>> requestCycle,
>> >>> RequestParameters requestParameters)
>> >>> {
>> >>> String bookmarkablePageClass =
>> >>> requestParameters.getBookmarkablePageClass();
>> >>> Session session = requestCycle.getSession();
>> >>> Application application = session.getApplication();
>> >>> Class pageClass;
>> >>> try
>> >>> {
>> >>> pageClass =
>> >>> session.getClassResolver().resolveClass(bookmarkablePageClass);
>> >>> }
>> >>> catch (RuntimeException e)
>> >>> {
>> >>> return new
>> >>> WebErrorCodeResponseTarget(HttpServletResponse.SC_NOT_FOUND,
>> >>> "Unable to load Bookmarkable Page");
>> >>> }
>> >>>
>> >>> try
>> >>> {
>> >>> final IPageFactory pageFactory =
>> session.getPageFactory ();
>> >>> synchronized (session) {
>> >>> Page newPage = pageFactory.newPage(pageClass,
>> >>> new
>> >>> PageParameters(requestParameters.getParameters()));
>> >>> // the response might have been set in the
>> >>> constructor of
>> >>> // the bookmarkable page
>> >>> IRequestTarget requestTarget =
>> >>> requestCycle.getRequestTarget();
>> >>> // is it possible that there is already
>> another
>> >>> request target at
>> >>> // this point then the 2 below?
>> >>> if (!(requestTarget instanceof
>> IPageRequestTarget ||
>> >>> requestTarget instanceof IBookmarkablePageRequestTarget))
>> >>> {
>> >>> requestTarget = new
>> PageRequestTarget(newPage);
>> >>> }
>> >>> return requestTarget;
>> >>> }
>> >>> }
>> >>> catch (RuntimeException e)
>> >>> {
>> >>> throw new WicketRuntimeException("Unable to
>> instantiate
>> >>> Page class: "
>> >>> + bookmarkablePageClass + ". See below for
>> >>> details.", e);
>> >>> }
>> >>> }
>> >>>
>> >>>
>> >>> On 1/5/06, *Martijn Dashorst* < [EMAIL PROTECTED]
>> <mailto:[EMAIL PROTECTED]>
>> >>> <mailto:[EMAIL PROTECTED]
>> <mailto:[EMAIL PROTECTED]>>> wrote:
>> >>>
>> >>> Testcase:
>> >>>
>> >>> Take wicket examples, linkomatic.
>> >>>
>> >>> Put a breakpoint on the Page3 constructor. Click on the
>> pagelink
>> >>> to the Page3 page twice (on the same page) and see two
>> threads
>> >>> waiting.
>> >>>
>> >>> The synchronized block seems to be too late.
>> >>>
>> >>> Martijn
>> >>>
>> >>>
>> >>>
>> >>> On 1/5/06, *Martijn Dashorst* < [EMAIL PROTECTED]
>> <mailto:[EMAIL PROTECTED]>
>> >>> <mailto:[EMAIL PROTECTED]
>> <mailto:[EMAIL PROTECTED]>>> wrote:
>> >>>
>> >>> According to our limited test, BookmarkablePageTargets
>> are
>> >>> executed outside the synchronized block. This means
>> that any
>> >>> session access in the constructor of the Page is not
>> >>> synchronized, and will get you into trouble. This is
>> what
>> >>> happens in our application.
>> >>>
>> >>> Questions:
>> >>> - is this by design?
>> >>> - has anyone changed this behavior since december 16?
>> >>>
>> >>> Martijn
>> >>>
>> >>> -- Living a wicket life...
>> >>>
>> >>> Martijn Dashorst - http://www.jroller.com/page/dashorst
>> >>> <http://www.jroller.com/page/dashorst>
>> >>>
>> >>> Wicket 1.1 is out:
>> http://wicket.sourceforge.net/wicket-1.1
>> >>>
>> >>>
>> >>>
>> >>> -- Living a wicket life...
>> >>>
>> >>> Martijn Dashorst - http://www.jroller.com/page/dashorst
>> >>>
>> >>> Wicket 1.1 is out: http://wicket.sourceforge.net/wicket-1.1
>> >>>
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Living a wicket life...
>> >>>
>> >>> Martijn Dashorst - http://www.jroller.com/page/dashorst
>> <http://www.jroller.com/page/dashorst>
>> >>>
>> >>> Wicket 1.1 is out: http://wicket.sourceforge.net/wicket-1.1
>> >>
>> >>
>> >> -------------------------------------------------------
>> >> This SF.net email is sponsored by: Splunk Inc. Do you grep
>> through
>> >> log files
>> >> for problems? Stop! Download the new AJAX search engine that
>> makes
>> >> searching your log files as easy as surfing the web. DOWNLOAD
>> SPLUNK!
>> >> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
>> < http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click>
>> >> _______________________________________________
>> >> Wicket-develop mailing list
>> >> [email protected]
>> <mailto:[email protected]>
>> >> https://lists.sourceforge.net/lists/listinfo/wicket-develop
>> < https://lists.sourceforge.net/lists/listinfo/wicket-develop>
>> >>
>> >
>> >
>> > -------------------------------------------------------
>> > This SF.net email is sponsored by: Splunk Inc. Do you grep
>> through log
>> > files
>> > for problems? Stop! Download the new AJAX search engine that
>> makes
>> > searching your log files as easy as surfing the web. DOWNLOAD
>> SPLUNK!
>> > http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
>> < http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click>
>> > _______________________________________________
>> > Wicket-develop mailing list
>> > [email protected]
>> <mailto:[email protected]>
>> > https://lists.sourceforge.net/lists/listinfo/wicket-develop
>> >
>>
>>
>> -------------------------------------------------------
>> This SF.net email is sponsored by: Splunk Inc. Do you grep through
>> log files
>> for problems? Stop! Download the new AJAX search engine that makes
>> searching your log files as easy as surfing the web. DOWNLOAD
>> SPLUNK!
>> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
>> < http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click>
>> _______________________________________________
>> Wicket-develop mailing list
>> [email protected]
>> <mailto:[email protected]>
>> https://lists.sourceforge.net/lists/listinfo/wicket-develop
>>
>>
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files
> for problems? Stop! Download the new AJAX search engine that makes
> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> _______________________________________________
> Wicket-develop mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
>
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
