Bugzilla – Full Text Bug Listing
| Summary: | Race condition with ui callbacks | ||
|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Claudio Guglielmo <claudio.guglielmo> |
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P2 | ||
| Version: | 1.5 | ||
| Target Milestone: | 2.0 M3 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
Good catch, thanks for tracking this down! I currently don't see any problem with your workaround, but of course, it doesn't sound like the perfect solution. The problem is that we access shared state in the session store (ATTR_NEEDS_UICALLBACK) from different threads without synchronization. The obvious solution would be to synchronize this access in UICallBackServiceHandler. I'd be happy if we could avoid making this already complicated system even more complicated. Here's one quick idea to make it simpler and avoid this kind of problems completely, for the price of one additional ui request at the end of the a UICallBack "session": If we could accept this additional request, we could agree that only the UIThread (currently DisplayLCA) sends out status updates to the client. Whenever a callback request is answered, the client sends out a new ui request. The callback requests would not contain any protocol message anymore. I'm not sure what happens in case of a session timeout, though. Anyway, this is just a rough idea, this problem needs some more thought. Fixed in commit 67e60e587b211b4fa0ee90df0f30cdaa2170c048. The UICallBack implementation has been changed as proposed in comment 1: * the property "active" is now only rendered in ui responses to prevent the race conditions described in this bug * the client must always send a UI request when a callback request returns Currently, all callback responses contain a call operation to instruct the client to send a new UI request. I've opened bug 392974 to remove this redundant operation and require clients to send new ui requests unconditionally. |
In our project we extensively use eclipse jobs, in fact nearly every click on a rap component creates an eclipse job. Since scheduling a job automatically activates the ui callback, a lot of ui callback service calls are done. As far as I have seen in the code the UICallBackManager gets deactivated after such a service call (UICallBackServiceHandler.service()) and activated on regular calls (DisplayLCA.render()), if necessary. Since the ui callback request is not processed by the regular client request queue the activation and deactivation of the ui callback could happen concurrently, which could lead to following problem: The ui callback on server side is active but not on client side (UICallBack.js). Although every regular service call should activate the ui callback if necessary, it does not happen, because the server thinks it already is. From now on ui callbacks are broken, only reloading the site fixes the problem. Steps to reproduce: 1. Add a breakpoint in UICallBackServiceHandler#writeUICallBackDeactivation on line sessionStore.setAttribute( ATTR_NEEDS_UICALLBACK, Boolean.valueOf( actual ) ); 2. Establish a ui callback connection or somehow execute UICallBackServiceHandler#service() 3. Add a breakopint in UICallBackServiceHandler#writeUICallBackActivation on line sessionStore.setAttribute( ATTR_NEEDS_UICALLBACK, Boolean.valueOf( actual ) ); 4. Click somewhere on the gui. 5. Step further with the first thread waiting in the method writeUICallBackDeactivation. Do NOT let it run. 6. Let the second thread run so the request goes back to the client. 7. Let the first thread run. Now the ui callback is activated on server side but deactivated on client side. To solve this problem I patched the rap bundle for our project and simply removed the check if it's already activated in writeUICallBackActivation. boolean actual = UICallBackManager.getInstance().needsActivation(); // boolean preserved = getPreservedUICallBackActivation(); if(actual ) { Do you think, this could lead to any other problems? Thank you in advance to have a look at this bug.