RFR: Finalizer support
Arthur Eubanks
aeubanks at google.com
Thu Sep 12 18:50:53 UTC 2019
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!
More information about the tsan-dev
mailing list