Fix potential misuse of the DFA start state when initializing a decision from multiple threads

Fixes #804
This commit is contained in:
Sam Harwell 2015-01-26 15:50:12 -06:00
parent 8d9c7d2801
commit 84fb456aac
2 changed files with 48 additions and 26 deletions

View File

@ -369,17 +369,12 @@ public class ParserATNSimulator extends ATNSimulator {
// But, do we still need an initial state?
try {
DFAState s0;
if (dfa.isPrecedenceDfa()) {
// the start state for a precedence DFA depends on the current
// parser precedence, and is provided by a DFA method.
s0 = dfa.getPrecedenceStartState(parser.getPrecedence());
}
else {
// the start state for a "regular" DFA is just s0
s0 = dfa.s0;
}
if (s0 == null) {
/* Threading note: dfa.s0 is the final value set on the DFA instance
* which affects branch choice in this method. If dfa.s0 is null
* (i.e. it has not been set), then we take the "slow" path through
* the initialization code to ensure a correct start state is used.
*/
if (dfa.s0 == null) {
if ( outerContext ==null ) outerContext = ParserRuleContext.EMPTY;
if ( debug || debug_list_atn_decisions ) {
System.out.println("predictATN decision "+ dfa.decision+
@ -391,8 +386,16 @@ public class ParserATNSimulator extends ATNSimulator {
* to determine if this ATN start state is the decision for the
* closure block that determines whether a precedence rule
* should continue or complete.
*
* Threading note: cannot check the dfa.isPrecedenceDfa()
* condition here, because it's possible for that value to
* return true prior to dfa.s0 being set to the precedence start
* state. Calling dfa.setPrecedenceDfa(true) multiple times is
* safe, and the one-time performance overhead only occurs if
* multiple threads try to initialize the same DFA start state
* at the same time.
*/
if (!dfa.isPrecedenceDfa() && dfa.atnStartState instanceof StarLoopEntryState) {
if (dfa.atnStartState instanceof StarLoopEntryState) {
if (((StarLoopEntryState)dfa.atnStartState).precedenceRuleDecision) {
dfa.setPrecedenceDfa(true);
}
@ -419,6 +422,19 @@ public class ParserATNSimulator extends ATNSimulator {
s0 = addDFAState(dfa, new DFAState(s0_closure));
dfa.s0 = s0;
}
} else if (dfa.isPrecedenceDfa()) {
/* Threading note: if dfa.s0 was not null, then
* dfa.isPrecedenceDfa() must have already been set to the
* correct value.
*/
// the start state for a precedence DFA depends on the current
// parser precedence, and is provided by a DFA method.
s0 = dfa.getPrecedenceStartState(parser.getPrecedence());
}
else {
// the start state for a "regular" DFA is just s0
s0 = dfa.s0;
}
int alt = execATN(dfa, s0, input, index, outerContext);

View File

@ -165,21 +165,27 @@ public class DFA {
* @param precedenceDfa {@code true} if this is a precedence DFA; otherwise,
* {@code false}
*/
public final synchronized void setPrecedenceDfa(boolean precedenceDfa) {
if (this.precedenceDfa != precedenceDfa) {
this.states.clear();
if (precedenceDfa) {
DFAState precedenceState = new DFAState(new ATNConfigSet());
precedenceState.edges = new DFAState[0];
precedenceState.isAcceptState = false;
precedenceState.requiresFullContext = false;
this.s0 = precedenceState;
public final void setPrecedenceDfa(boolean precedenceDfa) {
// Synchronize on this.states since we alter the map and other code
// which updates the DFA synchronizes on states, not on the DFA instance
// itself.
synchronized (this.states) {
if (this.precedenceDfa != precedenceDfa) {
this.states.clear();
// must set precedenceDfa before s0 to ensure correct branches
// are followed in adaptivePredict
this.precedenceDfa = precedenceDfa;
if (precedenceDfa) {
DFAState precedenceState = new DFAState(new ATNConfigSet());
precedenceState.edges = new DFAState[0];
precedenceState.isAcceptState = false;
precedenceState.requiresFullContext = false;
this.s0 = precedenceState;
}
else {
this.s0 = null;
}
}
else {
this.s0 = null;
}
this.precedenceDfa = precedenceDfa;
}
}