[OpenJDK 2D-Dev] RFR: 8271718: Crash when during color transformation the color profile is replaced

[Sergey Bylokhov]

On Sat, 7 Aug 2021 07:57:22 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

> I have started the investigation of this code after this comment: https://github.com/openjdk/jdk/pull/2957#discussion_r592988443
> 
> We have a suspicious synchronized keyword on the "setTagData" method. This method modified the data of the profile and invalidates the pointer to the native part of the profile. The usage of synchronized looks strange here since the code used by this method is thread-safe by itself.
> 
> I have double-checked the usage of the native pointers and found that a long time ago in jdk7 most of the methods in this class were synchronized and that prevents the usage of broken pointers, but unfortunately, the method "createTransform" added by the JDK-7043064 was not marked as such. So since then, it is possible to crash the "createTransform" by changing the content of the profile after "createTransform" save it locally and before it passes it to "the createNativeTransform".
> 
> There are three ways to fix the problem:
>  1. Mark "createTransform" by the "synchronized" keyword. But it will prevent calling transformation by the different threads in parallel.
>  2. Reimplement synchronization based on one static read/write lock, so modification of the profile could be possible by one thread at a time, while transformation could be done by a few threads. But the transformation will be blocked during some unused profiles(any profiles) modification.
>  3. Reimplement synchronization based on read/write lock per profile. So if some unused profiles will be modified the transformation will work fine. But each transform operation will have to lock each needed profile before proceeding and the number of profiles could be some N.
> 
> I have selected the second solution based on the next assumption:
> _The profile modification operation is rare, and the transform operation is much more common. So in the worst case, this change adds one additional call to lock/unlock to the transform and does not affect the "setTagData"_

src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMS.java line 95:

> 93:         } finally {
> 94:             lock.unlockWrite(stamp);
> 95:         }

We have only one LCMS object at a time, so using "synchronized" and "writeLock" are kind of equivalent.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5042