On Mon, 8 Jun 2026 14:57:53 GMT, Andy Goryachev <[email protected]> wrote:
>> ### Summary
>>
>> This PR makes deprecates the `DataFormat` constructor (for removal):
>>
>> public 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::l...
>
> Andy Goryachev has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains nine additional
> commits since the last revision:
>
> - Merge branch 'master' into 8373452.data.format
> - Merge branch 'master' into 8373452.data.format
> - 2026
> - Merge branch 'master' into 8373452.data.format
> - Merge branch 'master' into 8373452.data.format
> - junit
> - whitespace
> - javadoc
> - data format
I haven't filed one yet because I'm not sure yet which part of the issue is the
actual bug. I also can't reproduce it yet. If you look at the mailing list
reply of @mstr2, there is a reentrant issue when calling mimesFromSystem as it
calls gtk_main_iteration(). This means that when mimesFromSystem is called in
an event handler, the gtk_main_iteration can send other events while we are
still in the first event handler. In some cases this leads to an exception, but
will happen much more often without exceptions (although this is also probably
wrong).
-------------
PR Comment: https://git.openjdk.org/jfx/pull/2006#issuecomment-4651032252
