On Fri, 8 Nov 2024 11:56:08 GMT, Quan Anh Mai <qa...@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
