On Sat, 7 Aug 2021 07:57:22 GMT, Sergey Bylokhov <s...@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
