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"_

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

Commit messages:
 - Initial fix of JDK-8271718

Changes: https://git.openjdk.java.net/jdk/pull/5042/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5042&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8271718
  Stats: 165 lines in 2 files changed: 155 ins; 3 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5042.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5042/head:pull/5042

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

Reply via email to