Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verbesserung der thread-safety im ConfigurationBasedValidationModule #9

Open
dscheffer opened this issue Nov 10, 2023 · 2 comments
Open

Comments

@dscheffer
Copy link

dscheffer commented Nov 10, 2023

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
Copy link
Contributor

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
Copy link

DarthGizka commented Nov 13, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants