RFR: 8305425: Thread.isAlive0 doesn't need to call into the VM [v3]

[Aleksey Shipilev]

On Mon, 3 Apr 2023 22:41:44 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> We have the strange situation where calling `t.isAlive()` on a `java.lang.Thread` `t`, will call into the VM (via `alive()` then `isAlive0()`) where the VM then examines the `eetop` field of `t` to extract its `JavaThread` pointer and compare it to null. We can simply read `eetop` directly in `Thread.alive()`:
>> 
>> boolean alive() {
>>   return eetop != 0;
>> } 
>> 
>> I also updated a comment in relation to `eetop`.
>> 
>> Testing: tiers 1-3
>> 
>> Thanks
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Ensure HB relationship exists; additional explanatory comments

src/java.base/share/classes/java/lang/Thread.java line 231:

> 229:     /* Reserved for exclusive use by the JVM. Cannot be moved to the FieldHolder
> 230:        as it needs to be set by the VM before executing the constructor that
> 231:        will create the FieldHolder. The historically named `eetop`holds the address

Suggestion:

       will create the FieldHolder. The historically named `eetop` holds the address

src/java.base/share/classes/java/lang/Thread.java line 1850:

> 1848:      * happens-before isAlive() returning false.
> 1849:      */
> 1850:     synchronized boolean alive() {

Ah. Yes, it needs to be synchronized on the same lock that VM uses, which is `this`.

But there is also a secondary issue fixed by this synchronization: without it, a simple `while (t.isAlive()) {}` loop would execute indefinitely, since `eetop` is going to be hoisted. This can be reproduced with the test below.

Consider adding it as `test/jdk/java/lang/Thread/IsAlive.java`:


/*
 * Copyright 2023, Amazon.com Inc. or its affiliates. All Rights Reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2 only, as
 * published by the Free Software Foundation.
 *
 * This code is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 * version 2 for more details (a copy is included in the LICENSE file that
 * accompanied this code).
 *
 * You should have received a copy of the GNU General Public License version
 * 2 along with this work; if not, write to the Free Software Foundation,
 * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
 *
 * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
 * or visit www.oracle.com if you need additional information or have any
 * questions.
 */

/*
 * @test
 * @bug     8305425
 * @summary Check Thread.isAlive
 * @run main/othervm/timeout=10 IsAlive
 */

public class IsAlive {

    private static void spin() {
        try {
            while (!Thread.currentThread().isInterrupted()) {
                Thread.sleep(10);
            }
        } catch (InterruptedException ie) {
            // Do nothing, just exit
        }
    }

    static volatile boolean checkerReady;

    private static void check(Thread t) {
        while (!t.isAlive()) {
            // Burn hard, without any sleeps.
            // Check that we discover the thread is alive eventually.
        }

        checkerReady = true;

        while (t.isAlive()) {
            // Burn hard, without any sleeps.
            // Check that we discover the thread is not alive eventually.
        }
    }

    private static void assertAlive(Thread t) {
        if (!t.isAlive()) {
            throw new IllegalStateException("Thread " + t + " is not alive, but it should be");
        }
    }

    private static void assertNotAlive(Thread t) {
        if (t.isAlive()) {
            throw new IllegalStateException("Thread " + t + " is alive, but it should not be");
        }
    }

    public static void main(String args[]) throws Exception {
        Thread spinner = new Thread(IsAlive::spin);
        spinner.setName("Spinner");
        spinner.setDaemon(true);

        Thread checker = new Thread(() -> check(spinner));
        checker.setName("Checker");
        checker.setDaemon(true);

        assertNotAlive(spinner);
        assertNotAlive(checker);

        System.out.println("Starting spinner");
        spinner.start();
        assertAlive(spinner);

        System.out.println("Starting checker");
        checker.start();
        assertAlive(checker);

        System.out.println("Waiting for checker to catch up");
        while (!checkerReady) {
            Thread.sleep(100);
        }

        System.out.println("Interrupting and joining spinner");
        spinner.interrupt();
        spinner.join();
        assertNotAlive(spinner);

        System.out.println("Joining checker");
        checker.join();
        assertNotAlive(checker);

        System.out.println("Complete");
    }
}

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13287#discussion_r1156901698
PR Review Comment: https://git.openjdk.org/jdk/pull/13287#discussion_r1156973465