Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 316388

Summary: ExecutionObserver doesn't cancel executions when losing connection
Product: [Tools] PTP Reporter: Roland Schulz <roland>
Component: Remote ToolsAssignee: Greg Watson <g.watson>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 Flags: g.watson: review? (roland)
Version: 4.0   
Target Milestone: 4.0.1   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
patch to cancel executions after connection fails
none
revised patch
none
patch to sync correctly none

Description Roland Schulz CLA 2010-06-09 17:28:53 EDT 
The ExecutionObserver run method doesn't do anything with the executions if fConnection.isConnected() is false. Thus the waitForEndOfExcution waits forever. 

The attached patch cancels those executions. For the resource manager this seem to work correct. When the connection is lost, one gets the proper message that the server finished with -1. 

But I'm not sure whether this is how it is intended. E.g. should the ExecutionObserver itself be canceled if the connection is lost?
Comment 1 Roland Schulz CLA 2010-06-09 17:29:22 EDT 
Created attachment 171582 [details]
patch to cancel executions after connection fails
Comment 2 Greg Watson CLA 2010-06-14 20:32:10 EDT 
I'd say the ExecutionObserver should exit if the connection is lost. It is restarted each time connect() is called.

I've simplified the patch and removed the syncronize on fConnection.
Comment 3 Greg Watson CLA 2010-06-14 20:32:43 EDT 
Created attachment 171887 [details]
revised patch
Comment 4 Roland Schulz CLA 2010-06-14 20:53:03 EDT 
(In reply to comment #3)
> Created an attachment (id=171887) [details]
> revised patch

This doesn't solve the problem as far as I can see. Your modified patch doesn't call notifyFinish or notifyCancel for those KillableExecutions which are still running when the connection breaks up. Because you just leave the loop in this case those executions get never notified.
Comment 5 Greg Watson CLA 2010-06-15 10:03:15 EDT 
Oops, forgot about that.  Taking a look at this again, I see all sorts of synchronization problems with accessing activeProcessTable. I've redone the patch to synchronize these accesses. See what you think.
Comment 6 Greg Watson CLA 2010-06-15 10:04:22 EDT 
Created attachment 171933 [details]
patch to sync correctly
Comment 7 Greg Watson CLA 2010-06-22 10:39:14 EDT 
Roland, would you please review this patch? I'd like to get it into 4.0.1.
Comment 8 Roland Schulz CLA 2010-06-30 00:30:56 EDT 
(In reply to comment #7)
> Roland, would you please review this patch? I'd like to get it into 4.0.1.

Sorry. Was busy writing proposal. Hope it is not to late for 4.0.1.

The patch looks good.
Comment 9 Greg Watson CLA 2010-06-30 13:01:42 EDT 
Fixed in 4.0 and HEAD.