RFR 8224875: Shenandoah: ParallelCleaning code unloading should take lock to protect shared code roots array

[Aleksey Shipilev]

On 5/29/19 4:30 PM, Zhengyu Gu wrote:
>> How much does it really hurt for us to be on a safe side and grab the lock on add_* path too? We
>> would normally enter there with CodeCache lock held, and this might just protect us from some rare
>> corner case.
> 
> We can make it symmetric, but it does nothing :-)

Please do. We *think* it does nothing now, because we don't see how we can enter add_nmethod without
holding the CodeCache_lock. But do understand that is a flaky temporary understanding that can
change at any moment. The code should fail safe when that happens.

> ShenandoahLocker locker(CodeCache_lock->owned_by_self() ? NULL : &_recorded_nms_lock);
> It should always pass NULL to locker.

...and that is a problem how? The same happens in remove_nmethod.

> How about enhancing assertions and comments, clearly states the purpose of the lock.
> http://cr.openjdk.java.net/~zgu/JDK-8224875/webrev.02/
No, that is somehow worse. What prevents us to put this to both methods, and be done?

  assert_locked_or_safepoint(CodeCache_lock);
  ShenandoahSpinLocker locker(CodeCache_lock->owned_by_self() ? NULL : &_recorded_nms_lock);


Thanks,
-Aleksey