On 7/04/2021 6:37 pm, Robbin Ehn wrote:
On Wed, 31 Mar 2021 06:08:49 GMT, David Holmes <dhol...@openjdk.org> wrote:
Robbin Ehn has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains two commits:
- Merge branch 'master' into SuspendInHandshake
- 8257831: Suspend with handshake (review baseline)
src/hotspot/share/runtime/objectMonitor.cpp line 411:
409: OrderAccess::storestore();
410: for (;;) {
411: current->set_thread_state(_thread_blocked);
So IIUC because ThreadblockInVM is now responsible for handling thread
suspension checks, but those checks need to be done at a lower level, we can no
longer use ThreadBlockInVM in some places. That both undermines the value of
ThreadBlockInVM and make this manual management code much more complex.
I agree with Richard that a TBIVM that took an unlock function would make this
kind of code much clearer.
The problem is the we lock the OM from blocked state and then regret it.
Since we should not execute 'bytecode' during safe state but this
implementation does that we end up in this mess.
If this implementation did the correct thing we could use ThreadblockInVM
straight forward.
As I suggested if we are only blocked around parker it fixes it.
Pushing the thread-state transition closer to the actual part where we
park/block might seem to avoid this issue, but if we were unparked and
find ourselves suspended then we still have to wake another waiter,
which we aren't allowed to do without owning the lock so ...
The reason for grabbing the lock is that we cannot call exit on the OM which
wakes next thread.
Only the owner may call exit, this makes succession more complicate because you
might wake a suspended thread.
Now the suspended thread needs to wake another thread without owning the lock.
... moving the scope of the TBIVM doesn't fix it.
You've made the TBIVM responsible in part for suspension checks but the
scope of the TBIVM as used and the scope where we need suspension checks
don't match.
And yes adding a method ThreadBlockInVM can help here.
So bottom line is that we can define a new TBIVM that can work in these
cases and hide all the explicit state machinations?
Thanks,
David
-------------
PR: https://git.openjdk.java.net/jdk/pull/3191
