eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 721 forks source link

Possible issue with clone() #14401

Open gacholio opened 2 years ago

gacholio commented 2 years ago

Noticed while implementing #13192

The order of operations for a clone is:

The extremely unlikely but possible issue here is that the object allocation will report the JVMTI VMObjectAlloc event if it has been requested. This allows the jobject to potentially become visible to other threads while the rest of the clone is taking place.

Two problems here with the lockword:

Another issue is that another thread could see a torn long/double values in compressed refs mode as the cloning is not 64-bit atomic.

I'm not sure if there are any problems with:

Note that the JVMTI SampledObjectAlloc event has the same issue with the finalizer/synchronizer lists. This applies to all allocations, not just clone.

Note that a GC can occur when the JVMTI event is sent (between the allocate and the start of the field copy).

Practically speaking, no application should ever do this, but we've seen stranger things.

gacholio commented 2 years ago

@tajila @dmitripivkine @amicic

tajila commented 2 years ago

Is it possible to do the allocation without firing the SampledObjectAlloc event?

tajila commented 2 years ago

My understanding is that SampledObjectAlloc is conditional and is not triggered on every allocation, if thats true this should allow us to avoid the event in special cases

JasonFengJ9 commented 2 years ago

SampledObjectAlloc is enabled via https://github.com/eclipse-openj9/openj9/blob/99077564bf9bd2a32409b5861e8b8687061cddc0/runtime/jvmti/jvmtiCapability.c#L377-L390

gacholio commented 2 years ago

Is it possible to do the allocation without firing the SampledObjectAlloc event?

At the moment, I don't think so, and I wouldn't be in favour of adding this as an allocation option for such an edge case.