AbstractClassPathModelFinder.findModels is not thread-safe #1506

AbstractClassPathModelFinder.findModels(boolean) is not thread-safe. Since DefaultClassPathModelProvider delegates every load(..) call to it, loading models via a shared ClassPathModelProvider from several threads can fail.

Affected: 3.0.0-M6 and current main (opennlp-core/opennlp-model-resolver).

if (this.models == null || reloadCache) {
  final List<URI> classpathModels = getMatchingURIs("*.bin", getContext());
  final List<URI> classPathProperties = getMatchingURIs("model.properties", getContext());

  this.models = new HashSet<>();      // published before it is filled
  for (URI model : classpathModels) {
    ...
    this.models.add(new ClassPathModelEntry(model, Optional.ofNullable(m)));
  }
}
return this.models;

Problems:

1. The (non-volatile) field is assigned an empty HashSet before it is populated. A second thread calling findModels(false) meanwhile sees models != null and gets an empty or partially filled set, so ClassPathModelLoader.load(..) does not find a model that is actually on the classpath.
2. The returned set is the live, mutable cache. A concurrent first call or reloadCache = true adds to a HashSet while another thread iterates over it in the loader, which can throw ConcurrentModificationException or miss entries.
3. Two threads that both see models == null scan the classpath twice and overwrite each other's set.

We hit this in an application server where a shared provider loads different models (sentence detector, tokenizer, POS tagger) lazily from concurrent requests: the first parallel loads randomly failed or found no model. As a workaround we serialize all calls to the provider with an external lock.

Suggested fix: build the set in a local variable and publish it only once it is complete, as an unmodifiable set via a volatile field (or an AtomicReference). Guard the scan with a lock, or use double-checked locking, so the classpath is scanned only once for concurrent first calls. Also document the thread-safety of ClassPathModelFinder and ClassPathModelProvider in their Javadoc. A test could start N threads behind a CountDownLatch and call findModels(false) on a fresh finder, asserting that every caller gets the complete set.