scala-ide/scala-ide — pull request #505 by dotta (2013-08-23 13:53:07+00:00)
Title: Made `ScalaSourceFile.createFromPath` implementation thread-safe

The former implementation of `ScalaSourceFile.createFromPath` was not
thread-safe, which was problematic because the method is called concurrently
from different threads. The actual reason for the race-condition is that
`HandleFactory` is not thread-safe. (Side note, this is why I was keeping
getting "Resource XXX does not exist." exceptions while reviewing scala-search
PR #68).

The fix consists in making sure that no `HandleFactory` instance is shared
across threds. One relevant point is that `HandleFactory` performs caching for
speeding up retrieving an `Openable` from a `path` (the caching is the actual
reason why the class isn't thread-safe). This is an important aspect to keep in
mind, and which should motivate the implemented fix.

I could see two possible ways to fix `ScalaSourceFile.createFromPath`:

1) Create a fresh instance each time (and also provide an additional method to
allow callers to explictly pass a `HandleFactory` when they know the instance
isn't shared among threads).

2) Delegate the responsability of never sharing `HandleFactory` instances among
threads to a `ThreadLocal`.

Option 2) seemed to be the best fix, because

* correctness is ensured by design,
* no changes are required from the caller, and
* it maximize opportunities for re-using the cache maintained by `HandleFactory`.

Last but not least, I've taken the liberty of breaking both source and binary
compatibility. I believe the `handleFactory` instance was public for historical
reasons, and I don't expect anyone to complain about this breakage. However, I
could also re-introduce it, making it a `def`, mark it @deprecated, and simply
return a fresh `HandleFactory` instance.

Fixes #1001846