Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 359955 - Subsystem#connect() does not look thread-safe
Summary: Subsystem#connect() does not look thread-safe
Status: NEW
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.2.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: dsdp.tm.rse-inbox CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-05 05:33 EDT by Simon Bernard CLA
Modified: 2011-10-05 05:33 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Bernard CLA 2011-10-05 05:33:41 EDT
In SubSystem#connect(org.eclipse.core.runtime.IProgressMonitor, boolean) method, there is some code which looks like an attempt to implement thread safety, but I think this does not work.
A connectorServicePool seems to be here to manage the case where the current subsystem tries to connect, while an other one is connecting.

But I think there are some problems with that :

1. We are in the case where the connectorservice is connecting so the connectorservicepool is empty. We have 2 threads (A and B), we execute a connect on SubSystem at the same time for 2 different subsystems.Thread A executes the line 2520: 

boolean alreadyConnecting = connectorServicePool.contains(connectorService);

So alreadyconnecting == false.
Just after that, thread A was interrupted and thread B executes getConnectorService().connect(monitor);
Then, it's thread A's turn again, and it executes this line too.
… So we will connect twice to the connectorService.

2. This kind of concurrency protection works only if the connector service is connected through a subsystem.

3. In ConnectorServicePool#waitUntilNotContained(IConnectorService), /!\ this code seems weird :

while (contains(cs) && Display.getCurrent() == null){ 

This means that the method is blocking only if we are not in the UI Thread.


I don't really what would be the best thing to do here, ... maybe have no thread synchronisation at subsystem level and move the connection test in the connect method of connectorService which is already threadsafe.