RFR: Finalizer support

[Man Cao]

Looks good! Thanks for fixing.

-Man


On Thu, Sep 12, 2019 at 11:51 AM Arthur Eubanks <aeubanks at google.com> wrote:

>
>
> On Mon, Sep 9, 2019 at 6:03 PM Man Cao <manc at google.com> wrote:
>
>> Apology for the delay.
>>
>> I don't see how the TSAN code is guarded by the build-time option. Does
>> it work with "--with-jvm-features=-tsan".
>>
> Fixed by adding a NOT_TSAN macro and only returning ThreadSanitizer when
> TSAN is enabled as a JVM feature, else always return false.
> Made sure it compiled with --with-jvm-features=-tsan.
>
>>
>> For checking whether TSAN is enabled, is it possible to use the following
>> directly? Then we don't need to change jvm.h/cpp and symbols-unix.
>>
>> ManagementFactory.getRuntimeMXBean().getInputArguments().contains("-XX:+ThreadSanitizer");
>>
>> In Finalizer.java
>> private static boolean TSAN_ENABLED = isTsanEnabled();
>> It is better to mark TSAN_ENABLED field as "final".
>>
>  Done.
>
>>
>> if (TSAN_ENABLED) {
>>   tsanFinalize();
>> }
>> I think we can keep the TODO for this code, that it could be moved to
>> when a batch of finalizers was queued.
>>
> Done.
>
>>
>>
>> In Finalizer.c, should it add this line?
>> #include "java_lang_ref_Finalizer.h"
>> I see other files in the directory have includes for corresponding .h
>> files.
>>
> Done.
>
>>
>> In NonRacyFinalizerLoopTest.java:
>> * @build AbstractLoop TsanRunner
>>
>> public static void main(String[] args) throws InterruptedException {
>>
>> We can remove "AbstractLoop" and "throws InterruptedException", right?
>>
>> Removed AbstractLoop.
> TsanRunner.runTsanTestExpectSuccess() throws an IOException, threw that
> instead.
>
>>
>> -Man
>>
>
> New webrev: http://cr.openjdk.java.net/~aeubanks/tsanfinalizer/webrev.01/
> Thanks for the thorough review!
>