Concurrency issue in VersionSpecificWorkerContextWrapper

[Boereck]

Hi,
after upgrading to HAPI 7.6.0 we got ConcurrentModificationExceptions in our validation test runs, which uses the same FhirValidator instance (which the documentation claims is thread safe) to validate many example resources in parallel. It was quite tricky to find the source of the error, due to the concurrent nature, but we are certain there is a problem in the VersionSpecificWorkerContextWrapper.

Describe the bug
The problematic function is validateCodeInValueSet:

hapi-fhir/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java

Lines 803 to 834 in a999cb3

	private IValidationSupport.CodeValidationResult validateCodeInValueSet(
	IBaseResource theValueSet,
	ConceptValidationOptions theValidationOptions,
	String theSystem,
	String theCode,
	String theDisplay) {
	IValidationSupport.CodeValidationResult result = myValidationSupportContext
	.getRootValidationSupport()
	.validateCodeInValueSet(
	myValidationSupportContext, theValidationOptions, theSystem, theCode, theDisplay, theValueSet);
	if (result != null && isNotBlank(theSystem)) {
	/* We got a value set result, which could be successful, or could contain errors/warnings. The code
	might also be invalid in the code system, so we will check that as well and add those issues
	to our result.
	*/
	IValidationSupport.CodeValidationResult codeSystemResult =
	validateCodeInCodeSystem(theValidationOptions, theSystem, theCode, theDisplay);
	final boolean valueSetResultContainsInvalidDisplay = result.getIssues().stream()
	.anyMatch(VersionSpecificWorkerContextWrapper::hasInvalidDisplayDetailCode);
	if (codeSystemResult != null) {
	for (IValidationSupport.CodeValidationIssue codeValidationIssue : codeSystemResult.getIssues()) {
	/* Value set validation should already have checked the display name. If we get INVALID_DISPLAY
	issues from code system validation, they will only repeat what was already caught.
	*/
	if (!hasInvalidDisplayDetailCode(codeValidationIssue) || !valueSetResultContainsInvalidDisplay) {
	result.addIssue(codeValidationIssue);
	}
	}
	}
	}
	return result;
	}

In this function .getRootValidationSupport().validateCodeInValueSet(...) is called to get a CodeValidationResult. Since our root ValidationSupport is a CachingValidationSupport, this may return a cached object. Later it is checked if the code is valid in the CodeSystem. If this is not the case, the issues will be added to the result from the ValueSet validation, which may be a cached object!
In this case multiple parallel validations may write to the issues list of the result, causing ConcurrentModificationExceptions when reading the issue list later.

In future versions the ValidationSupportChain will do the caching and provoke the same problem.

Fortunately, the fix is pretty staight forward. Since the validateCodeInValueSet returns a result object, in the

if (codeSystemResult != null) {
	//...
}

block, result can be assigned to a copy of result. This can be implemented in VersionSpecificWorkerContextWrapper for the moment, but ideally IValidationSupport.CodeValidationResult could provide a copy constructor, or a copy instance method in future. However this class is in the org.hl7.fhir.core project, so this would have to be added upstream.
Since all objects referenced from IValidationSupport.CodeValidationResult (apart from the list-fields) are immutable, it would suffice to do a shallow copy, and just create list copies of the list-fields.

If wanted, we can provide a PR to fix this issue. We already tried the change and we did not face any ConcurrentModificationException after the change. We could also back-port this issue to the 7.6.0 stream, we just need to know against which branch the PR should go.

To Reproduce
This behavior is extremely tricky to reproduce, due to the concurrent nature. However, when validating a resource using a code in a place with with binding to a value set, that is not actually included in any CodeSystem referenced in the ValueSet, and doing so over and over in parallel with the same validator object, this might be triggered.

Expected behavior
No ConcurrentModificationException and and no multiplying issues on code in VS validation failures.

Environment (please complete the following information):

• HAPI FHIR Version 7.6.0 (also tested on 7.7.14-SNAPSHOT)