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]> 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] > 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

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

Wicket 1.1 is out: http://wicket.sourceforge.net/wicket-1.1

Reply via email to