Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match_Timeout_Repetition_Throws(engine: NonBacktracking) timing out #65991

Closed
stephentoub opened this issue Mar 1, 2022 · 3 comments · Fixed by #66365
Closed

Match_Timeout_Repetition_Throws(engine: NonBacktracking) timing out #65991

stephentoub opened this issue Mar 1, 2022 · 3 comments · Fixed by #66365
Assignees
Labels
area-System.Text.RegularExpressions disabled-test The test is disabled in source code against the issue untriaged New issue has not been triaged by the area owner
Milestone

Comments

@stephentoub
Copy link
Member

This outer loop test is now running for a very long time and eventually the test suite times out. I expect this is due to 83e5cbc, and that it's doing exactly what it tried to do, which is keep execution in the inner loop as long as possible. That would mean, however, that we wouldn't exit out into the outer loop frequently enough to hit the timeout check:

// Now run the DFA or NFA traversal from the current point using the current state.
int finalStatePosition;
int findResult = currentState.NfaState is not null ?
FindFinalStatePositionDeltas<NfaStateHandler>(builder, input, ref i, ref currentState, ref matchLength, out finalStatePosition) :
FindFinalStatePositionDeltas<DfaStateHandler>(builder, input, ref i, ref currentState, ref matchLength, out finalStatePosition);
// If we reached a final or deadend state, we're done.
if (findResult > 0)
{
return finalStatePosition;
}
// We're not at an end state, so we either ran out of input (in which case no match exists), hit an initial state (in which case
// we want to loop around to apply our initial state processing logic and optimizations), or failed to transition (which should
// only happen if we were in DFA mode and need to switch over to NFA mode). If we exited because we hit an initial state,
// find result will be 0, otherwise negative.
if (findResult < 0)
{
if ((uint)i >= (uint)input.Length)
{
// We ran out of input. No match.
break;
}
// We failed to transition. Upgrade to DFA mode.
Debug.Assert(currentState.DfaState is not null);
NfaMatchingState nfaState = perThreadData.NfaState;
nfaState.InitializeFrom(currentState.DfaState);
currentState = new CurrentState(nfaState);
}
// Check for a timeout before continuing.
if (_checkTimeout)
{
DoCheckTimeout(timeoutOccursAt);
}

cc: @olsaarik

@ghost
Copy link

ghost commented Mar 1, 2022

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

This outer loop test is now running for a very long time and eventually the test suite times out. I expect this is due to 83e5cbc, and that it's doing exactly what it tried to do, which is keep execution in the inner loop as long as possible. That would mean, however, that we wouldn't exit out into the outer loop frequently enough to hit the timeout check:

// Now run the DFA or NFA traversal from the current point using the current state.
int finalStatePosition;
int findResult = currentState.NfaState is not null ?
FindFinalStatePositionDeltas<NfaStateHandler>(builder, input, ref i, ref currentState, ref matchLength, out finalStatePosition) :
FindFinalStatePositionDeltas<DfaStateHandler>(builder, input, ref i, ref currentState, ref matchLength, out finalStatePosition);
// If we reached a final or deadend state, we're done.
if (findResult > 0)
{
return finalStatePosition;
}
// We're not at an end state, so we either ran out of input (in which case no match exists), hit an initial state (in which case
// we want to loop around to apply our initial state processing logic and optimizations), or failed to transition (which should
// only happen if we were in DFA mode and need to switch over to NFA mode). If we exited because we hit an initial state,
// find result will be 0, otherwise negative.
if (findResult < 0)
{
if ((uint)i >= (uint)input.Length)
{
// We ran out of input. No match.
break;
}
// We failed to transition. Upgrade to DFA mode.
Debug.Assert(currentState.DfaState is not null);
NfaMatchingState nfaState = perThreadData.NfaState;
nfaState.InitializeFrom(currentState.DfaState);
currentState = new CurrentState(nfaState);
}
// Check for a timeout before continuing.
if (_checkTimeout)
{
DoCheckTimeout(timeoutOccursAt);
}

cc: @olsaarik

Author: stephentoub
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: 7.0.0

@olsaarik
Copy link
Contributor

olsaarik commented Mar 3, 2022

Ok seems we'll need to introduce some equivalent of the previous "leeway" thing to pop out of the inner loop occasionally. The check for "did we reach the end of input" could just additionally check if the inner loop was actually given the full length of the input and if not just loop back (but skip the NFA promotion).

@olsaarik
Copy link
Contributor

olsaarik commented Mar 3, 2022

I'll be making changes to these matching loops in the PR after #66038, so I can fix it at the same time.

@stephentoub stephentoub self-assigned this Mar 8, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 8, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.RegularExpressions disabled-test The test is disabled in source code against the issue untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants