search

gematik / app-referencevalidator

The reference validator is a tool to perform advanced validation of FHIR resources for TI applications and interoperability standards
https://fachportal.gematik.de/hersteller-anbieter/primaersysteme/referenzvalidator
Other
21 stars 4 forks source link

Verbesserung der thread-safety im ConfigurationBasedValidationModule #9

Open dscheffer opened 12 months ago

dscheffer commented 12 months ago

Hallo zusammen,

ich habe im Zuge eines Projekts im Kontext E-Rezept ValidationModule.java auf thread-safety geprüft. Zwar gibt es kein akutes Problem, ich wollte aber darauf hinweisen, dass ConfigurationBasedValidationModule.configuration weder synchronisiert noch immutable ist und somit potentiell bei zukünftigen Änderungen oder falscher Verwendung ein multi-threading Problem darstellen könnte.

ValidationModuleConfiguration ist nämlich mit der Lombok @Data annotation versehen, welche public getter und setter für alle Felder erzeugt. Hinzu kommt, dass die verwendeten Maps und Lists alle 'mutable' sind. Man kann also prinzipiell nach der Initialisierung des ConfigurationBasedValidationModules so etwas tun: validationModule.getConfiguration().getSupportedProfiles().put("...", "...");. Dadurch, dass der Zugriff der configuration nicht synchronisiert ist, kann es demnach zu inkonsistenten Zugriffen in unterschiedlichen Threads kommen.

Es wäre daher in meinen Augen besser, wenn configuration grundsätzlich unveränderlich ist, dann ist auch keine falsche Verwendung möglich und soweit ich das überblicke, wird configuration intern nach der Initialisierung so wie so nicht verändert.

Ich beziehe mich oben auf folgende Klasse:

https://github.com/gematik/app-referencevalidator/blob/cf1e5d0cf4bd6ceabfee8e29fc47c79e56042ace/commons/src/main/java/de/gematik/refv/commons/configuration/ValidationModuleConfiguration.java#L31C1-L94

alexey-tschudnowsky commented 11 months ago

Hallo @dscheffer ,

vielen Dank für den Hinweis. Sie haben mit Ihren Ausführungen absolut recht. Wir werden die ValidationModuleConfiguration-Klasse im nächsten Release entsprechend anpassen.

Mit freundlichen Grüßen Alexey Tschudnowsky

DarthGizka commented 11 months ago

Alexey & @dscheffer: ich habe auf chat.fhir.org hierzu das Thema Thread Safety vs Lombok @Data aufgemacht, da ich ebenfalls gerade an genau dieser Problematik arbeite

Momentan ist das größtenteils nur ein Plädoyer für weniger Lombok und mehr handgeschmiedetes Java. Aber ich habe auch schon extensive Deep Dives bzgl. Thread Safety im HAPI-Validator hinter mir, insb. FhirContext, wg. des weitreichenden Kopplungspotentials. Ein Erfahrungsaustausch kann hier für alle Beteiligten von Vorteil sein.