RFR: 8343793: Test java/foreign/TestMemorySession.java is timing out

[Maurizio Cimadamore]

On Fri, 8 Nov 2024 11:56:08 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Hi,
> 
> This patch fixes the deadlock in `TestMemorySession#testAcquireCloseRace`. The lock-step spin lock is implemented as with `lock` being an `AtomicInteger`:
> 
>     // Keep the 2 threads operating on the same scope
>     int k = lock.getAndAdd(1) + 1;
>     while (k != i * 2) {
>         Thread.onSpinWait();
>         k = lock.get();
>     }
> 
> Given the initial condition:
> 
>     Thread 1: i = 0
>     Thread 2: i = 0
>     lock: -2
> 
> The `lock` then undergoes the following operations:
> 
> 
> 
>         Thread 1          Thread 2         lock value
>       getAndAdd(1)                            -1
>                         getAndAdd(1)           0 -> Thread 2 then continues its next iteration, its i value is now 1
>                         getAndAdd(1)           1 -> Thread 2 reaches the next iteration before thread 1 has a chance to read the value 0
>           get()                                1 -> Thread 1 now cannot proceed because it missed the value 0
>                            get()               1 -> Thread 2 now cannot proceed because lock can never reach 2
> 
> 
> The solution is to not rely on the exact value of the lock but instead whether the lock has passed the expected value.
> 
> Please take a look, thanks a lot.

I'm playing with something like this:


@Test
    public void testAcquireCloseRace() throws InterruptedException {
        int MAX_ITERATIONS = 1000;
        AtomicInteger iteration = new AtomicInteger();
        boolean[] result = new boolean[1];
        MemorySessionImpl[] scopes = new MemorySessionImpl[MAX_ITERATIONS];
        for (int i = 0; i < MAX_ITERATIONS; i++) {
            scopes[i] = MemorySessionImpl.toMemorySession(Arena.ofShared());
        }

        // This thread tries to close the scopes
        Thread t1 = new Thread(() -> {
            int it = iteration.get();
            while (it < MAX_ITERATIONS) {
                while (true) {
                    try {
                        scopes[it].close();
                        // if we close successfully, the iteration is now completed
                        break;
                    } catch (IllegalStateException e) {
                        // scope is acquired, wait and try again
                        Thread.onSpinWait();
                    }
                }
                // wait for other thread to complete its iteration
                int prev = it;
                while (prev == it) {
                    Thread.onSpinWait();
                    it = iteration.get();
                }
            }
        });

        // This thread tries to acquire the scopes, then check if it is alive after an acquire failure
        Thread t2 = new Thread(() -> {
            int it = iteration.get();
            while (it < MAX_ITERATIONS) {
                while (true) {
                    try {
                        scopes[it].acquire0();
                    } catch (IllegalStateException e) {
                        // we failed to acquire, meaning the other thread has closed this scope
                        if (scopes[it].isAlive()) {
                            result[0] = true;
                        }
                        // this iteration is now completed
                        break;
                    }
                    // if we get here, acquire was successful, so we need to release...
                    scopes[it].release0();
                    // ... and then try again
                }
                // advance to next iteration
                it = iteration.addAndGet(1);
            }
        });

        t1.start();
        t2.start();
        t1.join();
        t2.join();
        assertFalse(result[0]);
    }


I think I find this logic a bit more direct:

* both thread agree on an iteration count (the atomic integer)
* thread A keeps trying closing a scope -- if it succeds, its iteration is done, and it waits for the next thread to be done
* thread B keeps acquiring and releasing a scope -- if it fails to acquire, that means that the scope is closed by the other thread, so this iteration is done, and we can update the shared iteration counter

What do you think?

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

PR Comment: https://git.openjdk.org/jdk/pull/21976#issuecomment-2464628207