On Fri, 12 Dec 2025 23:38:49 GMT, Andy Goryachev <[email protected]> wrote:
> ### Summary
>
> This PR makes the `DataFormat` constructor private:
>
> private DataFormat(@NamedArg("ids") String... ids)
>
>
> and replaces it with
>
> public static DataFormat of(String ... ids)
>
>
> ### Problem
>
> There seems to be several issues with DataFormat API and implementation
> discovered during a Clipboard-related code review:
>
> 1. `static DataFormat::lookupMimeType(String)` is not thread safe: while
> iterating over previously registered entries in the `DATA_FORMAT_LIST`
> another thread might create a new instance (DataFormat L227)
>
> 2. `public DataFormat(String...)` constructor might throw an
> `IllegalArgumentException` if one of the given mime types is already assigned
> to another `DataFormat`. The origin of this requirement is unclear, but one
> possible issue I can see is if the application has two libraries that both
> attempt to create a `DataFormat` for let's say `"text/css"`. Then, depending
> on the timing or the exact code path, an exception will be thrown for which
> the library(-ies) might not be prepared. The constructor is also not thread
> safe.
>
> 3. To avoid a situation mentioned in bullet 2, a developer would is typically
> call `lookupMimeType()` to obtain an already registered instance, followed by
> a constructor call if such an instance has not been found. An example of such
> code can be seen in webkit/UIClientImpl:299 - but even then, despite that
> two-step process being synchronized, the code might still fail if *some
> other* library or the application attempts to create a new instance of
> DataFormat, since the constructor itself is not synchronized.
>
> 4. `DataFormat(new String[] { null })` is allowed but makes no sense!
>
> Why do we need to have the registry of previously created instances? Unclear.
> My theory is that the DataFormat allows to have multiple mime-types (ids) -
> example being `DataFormat.FILES = new
> DataFormat("application/x-java-file-list", "java.file-list");` - and the
> registry was added to prevent creation of a `DataFormat` with just one id for
> some reason.
>
> What should be done?
> - find out why we need this registry in the first place i.e. what could
> happen if we have multiple DataFormat instances with overlapping ids.
> - if the registry is needed add a new factory method, something like
> `DataFormat::of(String ...)` which is properly synchronized. This method will
> be called by the constructor to retain the backward compatibility.
> - deprecate (possibly for removal) `DataFormat::lookupMimeType(String)`, or
> keep it but have it properly synchronized
>
> ### Da...
Removing the constructor is an incompatible change.
modules/javafx.graphics/src/main/java/javafx/scene/input/DataFormat.java line
133:
> 131: */
> 132: @Deprecated(since = "27")
> 133: private DataFormat(@NamedArg("ids") String... ids) {
This is an incompatible change. You will need to go through the usual process
for such changes where it is deprecated for removal in one release and removed
in a subsequent release.
-------------
PR Review: https://git.openjdk.org/jfx/pull/2006#pullrequestreview-3684486211
PR Review Comment: https://git.openjdk.org/jfx/pull/2006#discussion_r2710227940
