RVM

What is the point of this?

Do you want execStatus to be private?

We shouldn't have any public fields and be using getters and setters (although a setter for changing a thread's state would imo be sick, as the thread's state is for its internal book keeping).

The intention in the previous/current thread design is to keep the thread state private (although it's actually protected - pooey). The majority of state changes occur through changeThreadState, this is a routine that encodes what state we think we are going from and what state we are going to. A bug in the port to PPC of the PNT branch was that a routine was being called when the thread was in the wrong state. In the previous/current code this violation would have caused an assertion failure.

The intention in the old design was that the implicit finite state machine for a thread would be defined by transitions on the state. To find the transitions a search for changeThreadState should suffice.

Covering all the corner cases of the Java thread model is difficult and the FSM approach got us to a large degree of JSR166 success. It adds documentation and debug for when unexpected things occur. This meant that if new bugs/errors were reported or detected in the field, fixing them wasn't a big deal. It adds a lot of documentation (and self documentation) which can make porting easier.

In the thread model before the 2.9 refactorings an adhoc model was what was used and this led to both bugs and the system being undocumented. I would not like to see the work that went into a cleaner FSM approach undone (not least as it will increase the effort for maintenance work).

Is this closed now? There are substantially more assertions in the system now.

I don't think that having a changeThreadState() method would be helpful for native threads. There are very few places where execStatus gets changed. Adding a changeThreadState() method would just be superfluous noise. On the other hand, I'd be happy to see more (correct!) uses of assertAcceptableStates() and friends. But, unlike green threads where state transitions were mostly synchronous except for when the thread was in JNI, the state transitions in native threads are much more asynchronous - hence there are fewer things that can be correctly asserted. This is not a bug, but a direct outcome of native threads being more correct.

Green threads were broken. The FSM approach was especially broken - it didn't allow things that Java programs should be able to do, like suspend a parked thread. Are you suggesting that fixing those bugs constitutes a regression and that we should reintroduce those bugs into the current code because it would somehow help documentation?

The current state transitions are clearly documented. See the javadoc for RVMThread.

JSR-166 worked almost on the first try in native threads; the only bug had nothing to do thread state transitions. I was unclear about the need to release the java.lang.Thread lock, and so that led to some deadlocks. That's now fixed, and JSR-166 works. So JSR-166 is not a good example for arguing in favor of the old approach. Also - take a look at the implementation of park() and unpark(). They're simpler now, in large part because we're not using a FSM to implement them.

I've got a patch to add some more assertions - it's included here. I don't think there are any other outstanding places that don't assert when dealing with execStatus, but if I'm wrong, please let me know.

I'll hold off on committing this until I've got the crashes sorted out (or until I give up on the crashes, which, hopefully, won't have to happen).

I'm not sure I'll get chance to look into it for a few days. Wrt suspend transitioning to parked, suspend is deprecated, the first time a transition of this kind would happen on green threads I'd get an assertion telling me to think about it. In the pre-cleaned up green thread code, which resembled the pre-cleaned up native thread code, the park would probably complete but the state of the thread may well not have been handled gracefully (the interrupted flag, etc.). Of course this never really happened as park wasn't implemented in the pre-cleaned up green thread code, but the intention was to make sure we at least thought about the transitions and what it was going to mean to the house keeping for one of these events to occur.

When you have a chance, do take a look. In the process of adding support for debugging state transitions I further consolidated the management of execStatus.

Moving all unscheduled issues to 3.1. Please close or retarget to a different fix target as appropriate.

Defer to 3.1.1

bulk defer open issues to 3.1.2

Bulk defer to 3.1.3; not essential to address for 3.1.2.