[13] RFR(M): 8163511: Allocation of compile task fails with assert: "Leaking compilation tasks?"

[Erik Österlund]

Hi Tobias,

On 2019-03-06 11:50, Tobias Hartmann wrote:
> Hi Erik,
>
> On 05.03.19 09:09, Erik Osterlund wrote:
>> Liveness of objects only change in safepoints. So if you are in the vm thread state, and ask if a holder is alive, its result is stable.
>>
>> With concurrent class unloading, marking terminates in a safepoint. At that snapshot in time, it is determined what is alive (strongly and through finalizers) and dead (although it might be lazily evaluated during concurrent execution). When that safepoint is released, that view will be consistent.
>>
>> As there are seemingly no safepoint checks in this code, all liveness information will be consistent and none of the imagined races exist with concurrent class unloading. It behaves as if it was all done in a safepoint, even though it was not.
>>
>> The key is that the klass_holder liveness (as well as the jweaks you use) is determined using a phantom load via the access API. That phantom load gives consistent results in the vm thread state as-if reference processing was done in a safepoint. The phantom loads can lazily compute what the value would have been, had reference processing been done in a safepoint, as it has stable liveness information from that safepoint. Then concurrent reference processing races for producing the same values.
>>
>> Basically there is nothing to worry about here as far as I can see. Looks good.
> Thanks a lot for the thorough explanation and the review.
>
>> Although freeing the jweaks in one place instead of two would make the code easier to follow. Up to you though and I don’t need another webrev for that.
> I could only factor the freeing into a separate method and call it from CompileTask::free() and
> CompileTask::select_for_compilation(). But I don't think that would make the code easier to read.

Ah, okay.

> Relying on the assumption that either both _method_holder and _hot_method_holder are weak references
> or both are strong, I've slightly refactored the code in CompileTask::free():
>
> http://cr.openjdk.java.net/~thartmann/8163511/webrev.02

The new webrev looks good to me.

Thanks,
/Erik

>
> Best regards,
> Tobias