gematik/DEMIS-validation-service

Threading issues of JSON searialization and potential issue with default locale change

Closed this issue · 3 comments

In the FhirJsonService the IParser is cached in a field. There are multiple issues with this approach. Spring Beans are singletons by default. This means multiple HTTP requests will use the beans concurrently. The first problem is that the cached field is not volatile or an AtomicReference. But even when the field was volatile, the documentation of FhirContext.newJsonParser() states:

Thread safety: Parsers are not guaranteed to be thread safe. Create a new parser instance for every thread or every message being parsed/encoded.

Performance Note: This method is cheap to call, and may be called once for every message being processed without incurring any performance penalty

So using a cached instance of the parser is not only unsafe, but also unnecessary, since creating a new instance is supposedly cheap.

Additionally, there are two uses where the default Locale is changed (in FhirContextService and ValidationService ). Since the beans depend on each other and the change is only done during setup, there cannot be a concurrency issue. It would still be better to centralize the mechanism and use synchronization to avoid concurrency issues in future, which may cause a wrong locale to be set as the "original" default on concurrent use. This might e.g. be triggered by test cases running in parallel etc.

As an additional note: The FhirContextService seems unnecessary, since it only provides singleton instances. These instances can be provided directly to the Spring Boot framework directly without having to inject a wrapper service, which has to be queried. An application configuration would provide the functionality to create the objects and the Spring Boot framework takes care of the lifecycle of the instances.

Daare commented

Hi Boereck,

Thank you for creating an issue in and also for the pull request! We appreciate your contribution and the effort you put into helping us improve this project.

We are currently looking into the issue you reported and will do our best to address it as soon as possible. If we have any questions or need any further information from you, we will definitely reach out to you.

Thanks again for bringing this to our attention, and please don't hesitate to let us know if you have any other feedback or suggestions for our project.

Best regards
Daniel

Daare commented

Hi again,

Thank you for your contribution to this project! We have reviewed the issue and pull request you submitted, and we greatly appreciate your suggestions for improvement.

We are actively using the feedback provided to make necessary changes to this project. Your input has been invaluable in helping us enhance the project and make it even better. We are committed to addressing the issues raised and incorporating the suggested changes.

We are planning to implement these changes as soon as possible and will keep you updated on the progress.

If you have any further feedback or suggestions, please feel free to share. We appreciate your involvement in improving this project, and we look forward to your continued support.

Best regards

Hello @Boereck

thank you again for your advice about the issue and contribution, we have internally analyzed the problem and developed our own solution, based on your precious suggestions (resolved in #6 )

If you find anything else, please don't hesitate to open a new issue to help us improve the service.

Thank you again.