On Thu, 30 Jul 2026 14:51:30 GMT, Andy Goryachev <[email protected]> wrote:
>> Nir Lisker has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - Update copyright year >> - Moved implNote to method docs >> - Remove unneeded . > > modules/javafx.base/src/main/java/javafx/util/StringConverter.java line 35: > >> 33: /// >> 34: /// @implNote JavaFX's implementations are immutable (the same converter >> can be reused, except for >> 35: /// `DateTimeStringConverter`s that can only be reused on the same >> thread). Implementing classes are not required to be > > Can you add a similar warning to `DateTimeStringConverter` please? It isn't just `DateTimeStringConverter` that cannot be used concurrently. The same is true of `NumberStringConverter`. The docs for `NumberFormat` say this: > Number formats are generally not synchronized. It is recommended to create > separate format instances for each thread. If multiple threads access a > format concurrently, it must be synchronized externally. Since `NumberFormat` is not thread-safe, and an instance of `NumberFormat` is cached in `NumberStringConverter`, we cannot allow concurrent access of `NumberStringConverter` (or its subclasses). So either they need to be used from the same thread or the application needs to synchronize. It might be better to not give any guarantees of thread-safety in this class and just say something like this: /// @implNote JavaFX's implementations are immutable, but are not thread-safe: the same /// converter can be reused as long as all access are on the same thread or are synchronized. /// Implementing classes are not required to be immutable. This represents a behavioral change, which is fine, but means that when you update the CSR, you will need to check the "Behavioral" box in the Compatibility risk section and also talk about the (minor) risk. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1880#discussion_r3684680351
