fix: match actor selection wildcards without backtracking - #3506
Open
pjfanning wants to merge 2 commits into
Open
fix: match actor selection wildcards without backtracking#3506pjfanning wants to merge 2 commits into
pjfanning wants to merge 2 commits into
Conversation
Motivation: SelectChildPattern compiled its glob to a regular expression with Helpers.makePattern, which turns every '*' into '.*'. A chain of those backtracks: the 26 character pattern "*a*a*a*a*a*a*a*a*a*a*a*a*b" matched against a 36 character actor name that cannot satisfy the trailing literal takes about 42 seconds on one thread. An ActorSelectionMessage carries its pattern elements in the message, so that cost is reachable from one small message, and deliverSelection runs on the caller's thread for a local selection and on the inbound stream thread for a remote one. Bounding the pattern length would not help, since 26 characters is already enough. Modification: Add Glob (@internalapi), which matches the same grammar - '?' is one character, '*' is any run, everything else is literal - by remembering only the most recent '*' rather than by backtracking, so it runs in time proportional to the product of the two lengths at worst. Match through it in ActorSelection.deliverSelection. SelectChildPattern.pattern is kept for compatibility but is now lazy, so deserializing a selection no longer compiles a regular expression either. Result: The same selections match as before, in time linear in practice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
SelectChildPatterncompiled its glob to a regular expression withHelpers.makePattern(
Helpers.scala:46), which quotes the literal runs but turns every*into.*. A chain ofthose backtracks badly. Measured on JDK 17, matching a pattern against a name of all
as thatcannot satisfy the trailing literal:
An
ActorSelectionMessagecarries its pattern elements in the message(
MessageContainerSerializer.scala:83-92), andSelectChildPattern's constructor compiles thepattern, so both the compile and the match are reachable from one small message.
ActorSelection.deliverSelectionruns on the caller's thread for a local selection and, for aremote one, directly on the inbound stream thread (
MessageDispatcher.scala:93, deliberately,"to make sure it is not stuck on busy user actor").
My first thought was to cap the pattern length in the serializer. The numbers above say that
would have been useless: 26 characters is already enough, and it is not a suspicious-looking
pattern. The cost has to come out of the matching itself.
Note that
untrusted-modedoes not cover this either — its check(
MessageDispatcher.scala:81-93) runs after deserialization, so it stops the matching but notthe compiling.
Modification
Add
org.apache.pekko.util.Glob(@InternalApi), which matches the same grammar —?isexactly one character,
*is any run, everything else is literal — with the standardsingle-backtrack-point wildcard algorithm: it remembers only the most recent
*and how muchit has consumed, so it never revisits a decision more than once per input position. Worst case
is proportional to the product of the two lengths; in practice it is linear.
ActorSelection.deliverSelectionnow matches throughSelectChildPattern.matches.SelectChildPattern.patternis kept for compatibility but is nowlazy, so deserializing aselection no longer compiles a regular expression at all.
Nothing else used
Helpers.makePattern, which is public API and is left in place.I did not add a cap on the number of selection elements. Once matching is linear the remaining
per-element cost is a small object allocation, already bounded by the frame size.
Result
The same selections match as before, in time linear in practice.
Tests
sbt "actor-tests/testOnly org.apache.pekko.util.GlobSpec"— 6 passed. The important one isagree with the regular expression it replaces, exhaustively over short inputs: every patternover
{a, b, *, ?}up to length 4 (341 of them) against every input over{a, b}up tolength 4, compared against
Helpers.makePattern. All agree, so the grammar is unchanged.sbt "actor-tests/testOnly org.apache.pekko.actor.ActorSelectionSpec"— 28 passed, including anew end-to-end test that sends a wildcard selection with the pathological pattern to a parent
with a 36 character child and asserts it returns in under 3 seconds.
42561317850 was not less than 3000000000— 42.5 seconds.sbt "remote/testOnly org.apache.pekko.remote.serialization.MessageContainerSerializerSpec"— 3 passedsbt "actor/mimaReportBinaryIssues"— no issuessbt "actor/scalafmtCheckAll" "actor-tests/scalafmtCheckAll" headerCreateAll— cleanReferences
None.
Glob.scalais new code and carries the standard ASF header.