<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Dear JavaFX developers:</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I wanted to as your opinion on this:</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<h3 style="direction: ltr; text-align: left; text-indent: 0px; line-height: 1.25; text-transform: none; background-color: rgb(255, 255, 255); margin-top: 24px; margin-bottom: 16px; font-family: -apple-system, system-ui, "Segoe UI", "Noto Sans", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 20px; font-weight: 600; color: rgb(31, 35, 40);">
<span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt;">Problem</span></h3>
<p style="direction: ltr; text-align: left; text-indent: 0px; text-transform: none; background-color: rgb(255, 255, 255); margin-top: 0px; margin-bottom: 16px;">
<span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40);">There seems to be several issues with DataFormat API and implementation discovered during a Clipboard-related code review:</span></p>
<ol start="1" style="direction: ltr; text-align: left; margin-top: 0px; margin-bottom: 16px; padding-left: 0px; background-color: rgb(255, 255, 255);">
<li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-left: 24px;">
<p role="presentation" style="direction: ltr; margin-top: 16px; margin-bottom: 16px;">
<span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">static DataFormat::lookupMimeType(String)</code></span><span style="text-transform: none;"> is not
thread safe: while iterating over previously registered entries in the </span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DATA_FORMAT_LIST</code></span><span style="text-transform: none;"> another
thread might create a new instance (DataFormat L227)</span></p>
</li><li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-top: 0.25em; margin-left: 24px;">
<p role="presentation" style="direction: ltr; margin-top: 16px; margin-bottom: 16px;">
<span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">public DataFormat(String...)</code></span><span style="text-transform: none;"> constructor might
throw an </span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">IllegalArgumentException</code></span><span style="text-transform: none;"> if one
of the given mime types is already assigned to another </span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat</code></span><span style="text-transform: none;">.
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
</span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat</code></span><span style="text-transform: none;"> for let's say
</span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">"text/css"</code></span><span style="text-transform: none;">. 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.</span></p>
</li><li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-top: 0.25em; margin-left: 24px;">
<p role="presentation" style="direction: ltr; margin-top: 16px; margin-bottom: 16px;">
<span style="text-transform: none;">To avoid a situation mentioned in bullet 2, a developer would is typically call
</span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">lookupMimeType()</code></span><span style="text-transform: none;"> 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
<i>some other</i> library or the application attempts to create a new instance of DataFormat, since the constructor itself is not synchronized.</span></p>
</li><li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-top: 0.25em; margin-left: 24px;">
<p role="presentation" style="direction: ltr; margin-top: 16px; margin-bottom: 16px;">
<span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat(new String[] { null })</code></span><span style="text-transform: none;"> is allowed but
makes no sense!</span></p>
</li></ol>
<p style="direction: ltr; text-align: left; text-indent: 0px; text-transform: none; background-color: rgb(255, 255, 255); margin-top: 0px; margin-bottom: 16px;">
<span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40);">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 </span><span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); background-color: rgba(129, 139, 152, 0.12);"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat.FILES
= new DataFormat("application/x-java-file-list", "java.file-list");</code></span><span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40);"> - and the registry was added to prevent creation of a
</span><span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); background-color: rgba(129, 139, 152, 0.12);"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat</code></span><span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40);"> with
just one id for some reason.</span></p>
<p style="direction: ltr; text-align: left; text-indent: 0px; text-transform: none; background-color: rgb(255, 255, 255); margin-top: 0px; margin-bottom: 16px;">
<span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40);">What should be done?</span></p>
<ul style="direction: ltr; text-align: left; margin-top: 0px; margin-bottom: 16px; padding-left: 0px; background-color: rgb(255, 255, 255);">
<li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-left: 24px;">
<span style="text-transform: none;" role="presentation">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.</span></li><li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-top: 0.25em; margin-left: 24px;">
<span style="text-transform: none;" role="presentation">if the registry is needed add a new factory method, something like
</span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;" role="presentation"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat::of(String ...)</code></span><span style="text-transform: none;" role="presentation"> which
is properly synchronized. This method will be called by the constructor to retain the backward compatibility.</span></li><li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-top: 0.25em; margin-left: 24px;">
<span style="text-transform: none;" role="presentation">deprecate (possibly for removal)
</span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;" role="presentation"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat::lookupMimeType(String)</code></span><span style="text-transform: none;" role="presentation">,
or keep it but have it properly synchronized</span></li></ul>
<h3 style="direction: ltr; text-align: left; text-indent: 0px; line-height: 1.25; text-transform: none; background-color: rgb(255, 255, 255); margin-top: 24px; margin-bottom: 16px; font-family: -apple-system, system-ui, "Segoe UI", "Noto Sans", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 20px; font-weight: 600; color: rgb(31, 35, 40);">
<span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt;">Dangers</span></h3>
<ol start="1" style="direction: ltr; text-align: left; margin-top: 0px; margin-bottom: 16px; padding-left: 0px; background-color: rgb(255, 255, 255);">
<li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-left: 24px;">
<span style="text-transform: none;" role="presentation">adding synchronization might lead to deadlocks if the application or library has existing code synchronized around some other object and not
</span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;" role="presentation"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat.class</code></span><span style="text-transform: none;" role="presentation">.</span></li><li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-top: 0.25em; margin-left: 24px;">
<span style="text-transform: none;" role="presentation">removing the public constructor is a very visible, breaking change</span></li></ol>
<h3 style="direction: ltr; text-align: left; text-indent: 0px; line-height: 1.25; text-transform: none; background-color: rgb(255, 255, 255); margin-top: 24px; margin-bottom: 16px; font-family: -apple-system, system-ui, "Segoe UI", "Noto Sans", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"; font-size: 20px; font-weight: 600; color: rgb(31, 35, 40);">
<span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt;">Alternatives</span></h3>
<p style="direction: ltr; text-align: left; text-indent: 0px; text-transform: none; background-color: rgb(255, 255, 255); margin-top: 0px; margin-bottom: 16px;">
<span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40);">We could possibly prevent the application code creating
</span><span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); background-color: rgba(129, 139, 152, 0.12);"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat</code></span><span style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40);">s
with multiple ids by</span></p>
<ul style="direction: ltr; text-align: left; margin-top: 0px; margin-bottom: 16px; padding-left: 0px; background-color: rgb(255, 255, 255);">
<li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-left: 24px;">
<span style="text-transform: none;" role="presentation">creating a </span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;" role="presentation"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">public
DataFormat(String)</code></span><span style="text-transform: none;" role="presentation"> constructor</span></li><li style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(31, 35, 40); margin-top: 0.25em; margin-left: 24px;">
<span style="text-transform: none;" role="presentation">allowing multiple instances with the same id by removing the registry or using the registry only for
</span><span style="background-color: rgba(129, 139, 152, 0.12); text-transform: none;" role="presentation"><code style="font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif;">DataFormat</code></span><span style="text-transform: none;" role="presentation">s
with multiple ids</span></li></ul>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Draft PR: <a href="https://github.com/openjdk/jfx/pull/2006" data-outlook-id="83fb8841-9f92-4b66-99bb-1058580df033">
https://github.com/openjdk/jfx/pull/2006</a></div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks,</div>
<div style="direction: ltr; font-family: "Iosevka Fixed SS16", Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
-andy</div>
</body>
</html>