experimenting with suspension of requestCycle via servlet 3 async #214
Conversation
Notes: - enabled async-supported in web.xml for ComponentReferenceApplication - LinkPage shows usage of RequestCycle#suspend() - pageAccessSynchronizer uses the requestCycle instead of the thread now, since two threads might process a page (PageAccessSynchronizerTest still fails) - ServletWebRequest must not call httpServletRequest#getContextPath(), since it returns null during async processing
| @@ -93,8 +94,8 @@ public Duration getTimeout(int pageId) | |||
| */ | |||
| public void lockPage(int pageId) throws CouldNotLockPageException | |||
| { | |||
| final Thread thread = Thread.currentThread(); | |||
| final PageLock lock = new PageLock(pageId, thread); | |||
| final RequestCycle cycle = RequestCycle.get(); | |||
There was a problem hiding this comment.
This is not a good idea.
Two threads should not work on the same page instance at the same time.
This will lead to concurrency related issues.
E.g. two Ajax requests will have their own Request Cycles but might work on the same page instance this way. Without AsyncContext being needed at all.
There was a problem hiding this comment.
Not true!
As long as one RequestCycle holds the lock on a page, no other RequestCycle can get a lock. It doesn't make any difference whether we lock on the thread or the RequestCycle.
There was a problem hiding this comment.
How a RequestCycle holds a lock on a page ?
Where is this lock in the code ?
There was a problem hiding this comment.
PageLock is the lock: as long as there is a PageLock instance, all access to the corresponding page has to wait.
Currently PageLock holds a reference to the thread, but the thread isn't used actually apart from identifying the lock. The RequestCycle can be used equally well for that.
There was a problem hiding this comment.
OK. I see!
But why do you need to change this code at all ?
PageLock#getCycle() is not used anywhere in the PR.
The thread is very useful to identify problems with long running requests at the moment. The thread stack trace is telling me much more than a long (the request cycle start time).
I'd prefer to add the RequestCycle as yet another field to PageLock than removing the thread. But I don't really see the need to change PageAccessSynchronizer at all.
There was a problem hiding this comment.
Sure, we could keep the thread reference. But when the request is suspended, a certain time no thread is keeping the lock (the page is still locked though) and from a certain point in time another thread would pick up the lock.
There was a problem hiding this comment.
In that timeframe the page should be unlocked. On resume the new thread should acquire a new lock or wait if it is not available at the moment.
There was a problem hiding this comment.
No I don't think so: The request processing hasn't finished yet (e.g. rendering the page hasn't been started yet), so the lock on the page cannot be released.
There was a problem hiding this comment.
Nothing from me - I didn't have any use for this yet, so I'm not sure how useful this is for components and how it should work exactly.
|
This PR seems to be inactive for too long :( |
Let's discuss how Wicket could support async processing.
Idea is to allow suspending the current RequestCycle, example's LinkPage shows the usage.
Notes:
, since two threads might process the same pages successively - is that a valid approach?(PageAccessSynchronizerTest has to be adapted)