Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 316388 - ExecutionObserver doesn't cancel executions when losing connection
Summary: ExecutionObserver doesn't cancel executions when losing connection
Status: RESOLVED FIXED
Alias: None
Product: PTP
Classification: Tools
Component: Remote Tools (show other bugs)
Version: 4.0   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 4.0.1   Edit
Assignee: Greg Watson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-09 17:28 EDT by Roland Schulz CLA
Modified: 2010-06-30 13:01 EDT (History)
0 users

See Also:
g.watson: review? (roland)

Attachments
patch to cancel executions after connection fails (1.46 KB, patch)
2010-06-09 17:29 EDT, Roland Schulz CLA 		no flags Details | Diff
revised patch (3.08 KB, patch)
2010-06-14 20:32 EDT, Greg Watson CLA 		no flags Details | Diff
patch to sync correctly (5.64 KB, patch)
2010-06-15 10:04 EDT, Greg Watson CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.