[equinox-dev] Thread Safety Issues

[Rob Harrop <rob.harrop@xxxxxxxxxxxxxxxx>]

Chaps,

I've created a few bug reports (with patches) related to thread safety issues we have found while using Equinox in SpringSource dm Server. I felt that it wasn't appropriate to create massive patches so I focused instead on point fixes, but I wanted to briefly outline what I think is a fuller solution to the highlighted issues. 

So far we have identified three main issues:

 * BaseStorage.getNextBundleId() performs unsafe increment
 	(https://bugs.eclipse.org/bugs/show_bug.cgi?id=247520)
 * StateImpl access bundleDescriptions in an unsafe manner
	(https://bugs.eclipse.org/bugs/show_bug.cgi?id=247522)
 * Framework access BundleRepository in an unsafe manner
	(https://bugs.eclipse.org/bugs/show_bug.cgi?id=247521)

For the first issue, I think the patch I supplied is fine, the solution there is pretty straightforward.

For the StateImpl issue, I attacked only the issue around accesses to the bundleDescriptions field, but I think that accesses to all fields need to be protected. Moreover, I think that all the state of StateImpl needs to be protected by the same lock to guarantee atomic updates of the internal data structures. 

In Framework, all calls to BundleRepository.add were not guarded by the bundles lock. I made sure that all accesses to bundles are correctly (I think) guarded. However, I feel a better approach is to make BundleRepository itself threadsafe. The JavaDoc of BR currently states that it must be synchronized before access but obviously it is easy to forget, as is the case currently. Also, I notice that Framework.bundles is used as a monitor for a lot of operations that don't directly manipulate its state and it can also escape control of Equinox via Framework.getBundles(). This is deadlock prone. Ideally, the BundleRepository would remain an Equinox internal data structure, maintain its own thread safetry and not be used as a monitor for other operations. Replacing the use of Framework.bundles for synchronization with something like Framework.bundlesMonitor will help here.

I haven't had to chance to explore the code further, but I'll raise any other issues as I find them.

If you want to make these larger thread-safety changes, I am happy to submit patches that address them.

Regards,

Rob

-- 
Rob Harrop
SpringSource