string.match() can trigger "unable to perform mandatory yield" error
complete
Jamesp1989 Resident
found another way to cause the bug found here https://feedback.secondlife.com/slua-alpha/p/unable-to-perform-mandatory-yield-error
it is happening with my script every now and then. not often enough to track down the part of the code that is triggering it. i have a script that reproduces the error fairly reliably
I'm pretty sure I can narrow it down to something with ll.GetNotecardLineSync as this started after moving over from ll.GetNotecardLine with data server event
To reproduce add a notecard called LootConfig to an object along side the below script and compile
-- Global variables to track line number and notecard query ID
notecardLine = 0
notecardQueryId = nil
--tables
testTable = {}
rw = {}
rc = {}
rMessage = {}
lStack = {}
config = {}
-- Price of the loot box (read from the notecard)
lPrice = 0
--current user
currentUser = nil
function read_notecard_sync(name)
local notecard = "LootConfig" -- Ensure this is a valid notecard name
local line = 0
if notecard and notecard ~= "" then -- Ensure the notecard name is valid
local text = ll.GetNotecardLineSync(notecard, line)
while text ~= EOF and text ~= NAK do
--ll.OwnerSay(text)
process_line(text)
line = line + 1
text = ll.GetNotecardLineSync(notecard, line)
end
if text == NAK then
ll.OwnerSay("Cache interrupted! getting data")
ll.GetNumberOfNotecardLines("LootConfig")
ll.ResetScript()
else
ll.OwnerSay("finished reading successfully")
if config["price"] then
lPrice = tonumber(config["price"])
ll.SetPayPrice(PAY_HIDE, { lPrice, PAY_HIDE, PAY_HIDE, PAY_HIDE })
ll.OwnerSay("Loot box price set to: " .. lPrice .. " L$")
updatePriceDisplay()
else
ll.OwnerSay("Warning: No price set in the notecard.")
end
end
else
ll.OwnerSay("Error: Notecard name is invalid or missing!")
end
end
function process_line(line)
local key, value = string.match(line, "([%w_]+)%s*=%s*(.+)")
if key and value then
local num_value = tonumber(value)
config[key] = num_value or value
return
end
local r, c, w, i, m = string.match(line, "([^:]+):([^:]+):([^:]+):([^:]+):(.+)")
if r and c and w and i and m then
local iList = string.split(i, ",")
testTable[r] = iList
rw[r] = tonumber(w) or 1
rc[r] = c
rMessage[r] = m
else
ll.OwnerSay("Failed to match line format: " .. line)
end
end
for i=1,1000,1 do
read_notecard_sync("LootConfig")
ll.ResetScript()
end
Log In
H
Harold Linden
complete
We've improved the error message and started allowing scheduler-injected sleeps to happen before
string.match()
and friends.A more complete solution to the problem of
string.match()
being un-yieldable will have to come at a later time as it would involve a large rewrite of string.match()
and friends that (to our knowledge) has no precedent.H
Harold Linden
We've got a fix internally that does two things
- We do a check right before the script enters a string.match()call or what have you to see if you're low on time left, and yield the script before entering to make sure we have time to run the function next time your script gets to run.
- We give a context-sensitive error to say whythe script scheduler had to rudely halt your script, like
lua_script:2: String pattern too complex, timed out
[C] function find
lua_script:2 function foo
lua_script:5
Note that we did decide to make this error catchable with
pcall()
in the general case, but if you're doing something incredibly weird like doing those calls inside multiple nested metamethods with multiple nested pcall()
s we will get increasingly aggressive about halting the script.That fix should go out sometime next week.
Jamesp1989 Resident
Harold Linden will the low error check cause a graceful pause and then retry or is this something that we will need to add to our scripts ?
H
Harold Linden
Jamesp1989 Resident It's a graceful pause initially, if it doesn't have enough time to run it when it comes back it will throw an
error()
because it'll likely never be able to run it within the time limits.That does introduce the problem that if the server is running too slow, or someone sends in a string longer than you expect, you might have patterns that are
right
on the edge of being killable that end up causing an error()
depending on their input or server load.As an example
local expensive_pattern = string.rep("a?", 50)..string.rep("a", 50)
string.find("a", expensive_pattern)
will run fine, but as input length increases the performance becomes worse,
string.find(string.rep("a", 50), expensive_pattern)
will definitely have to be killed.
We haven't come up with a great solution for that yet since it basically requires rewriting
g?match()
, gsub()
and find()
to support yielding internally, but hopefully the changes we're going to push out cover most of the non-pathological cases.Jamesp1989 Resident
Harold Linden yea making it wait for the time slice basically removed the error. i can still wrap it in a pcall and handle errors that might happen. considering the sleep is happening before each line read the only way it should still error is if someone puts a crazy amount of data on 1 line. the intended format is Name:Colour:Chance:item1,item2:string
i guess i could split the read further if needed but that shouldnt be enough to error with the full slice per line
H
Harold Linden
Jamesp1989 Resident That makes sense. We're also going to look at pulling in a regex library with better performance characteristics once we're further along in development,
match()
performance is hard to reason about at a glance even for me.H
Harold Linden
Jamesp1989 Resident
Is this behaving properly now if you recompile your script and remove the
ll.Sleep()
?Jamesp1989 Resident
Harold Linden il try tomorrow its 1am here. Is there a chance we could get change logs when yall push a new sim version ?
H
Harold Linden
Jamesp1989 Resident I'll ask about that, not sure how those release notes pages normally get created / published
Jamesp1989 Resident
Harold Linden it doesnt look like the luau viewer is having its sim updates pushed https://releasenotes.secondlife.com/repositories.html
Jamesp1989 Resident
Harold Linden struggling to ll.sleep myself so i came on to test
i still get it but the error is now
String pattern too complex, timed out without sleep and is now crashing even with sleep
H
Harold Linden
Jamesp1989 Resident That's strange, we didn't really change anything other than making
string.match()
and friends eligible for an implicitly injected sleep before they're run...All this is really making me think that we really need to rewrite Luau's
string.*
functions to properly allow yielding in the middle of matching, but I don't believe anyone's done that with either Luau or mainline Lua other than implementing much slower versions in native Lua code.For the moment, it's probably best to do a
string.split(some_val, ":")
and match over individual fields, merging any extra elements at the end back together if you want to allow the separator in those fields. The performance of patterns like "([^:]+):([^:]+):([^:]+):([^:]+):(.+)"
is pretty much guaranteed to be poor on longer strings given how much backtracking it has to do in the standard Lua implementations.Jamesp1989 Resident
Harold Linden would running our own string package as a modified version of the built in string package? But yea my fix was to do smaller patterns and have my = be a delimiter ratber than searching for the = in string. I was unable to get a error even with 50 lines being loaded huge difference.
H
Harold Linden
in progress
H
Harold Linden
tracked
Jamesp1989 Resident
its worth mentioning that this doesn't seem to repro or at least i havent been able to get it to even without the sleep
function process_line(line)
-- Handle key-value pairs (config entries)
local eq_pos = string.find(line, "=")
if eq_pos then
local key = string.sub(line, 1, eq_pos - 1):match("[%w_]+")
local value = string.sub(line, eq_pos + 1):match("^%s*(.-)%s*$")
if key and value then
local num_value = tonumber(value)
config[key] = num_value or value
return
end
end
-- Handle loot data
local r, c, w, i, m = line:match("([^:]+):([^:]+):([^:]+):([^:]+):(.+)")
if r and c and w and i and m then
local iList = {}
for item in i:gmatch("([^,]+)") do
table.insert(iList, item)
end
testTable[r] = iList
rw[r] = tonumber(w) or 1
rc[r] = c
rMessage[r] = m
else
ll.OwnerSay("Failed to match line format: " .. line)
end
end
for i = 1, 1000 do
read_notecard_sync("LootConfig")
end
H
Harold Linden
under review
H
Harold Linden
You're most likely to run into these when using
string.match()
or string.gsub()
, we have guards to prevent people from using patterns that eat up too much CPU time beyond what scripts are suppose to consume, because we have no way to interrupt and resume them if they take too long. Those functions are especially difficult to get right, because it's easy to get them to exhibit exponential slowdown given certain input lengths / patterns.https://love2d.org/forums/viewtopic.php?t=80128 has some background info on the issue in other uses of Lua.
For example (and this will run in the official Luau REPL as well:)
local line = string.rep("a ", 100) .. " = " .. string.rep("b ", 100)
local start = os.clock()
local r, c, w, i, m = string.match(line, "([^:]+):([^:]+):([^:]+):([^:]+):(.+)")
print(os.clock() - start)
prints
0.0018
on my local machine, which isn't great. As soon as you reach 0.002
seconds without being able to reach a yieldable point in your script, there's a high chance your script is going to get killed because it's not sharing its time with other scripts. It gets significantly worse if you prepend a %s*
to the pattern.By contrast
local key, value = string.match(line, "([%w_]+)%s*=%s*(.+)")
only takes 0.000024
.Writing pattern strings that don't exhibit exponential behavior is a bit of an art, and we should probably include a wrapper around a better regex lib that doesn't do backtracking,
string.match()
is a bit of a potential footgun in an environment that demands low latency, but we need it for compatibility.We might also be able to paper over this a little by forcing a yield just before
string.match()
and friends if you're getting close to your script's time limit. We let you go over it a little, but string.match()
's behavior is particularly bad.Jamesp1989 Resident
Harold Linden would wrapping in a coroutine help? Also this error didnt happen till i swapped to the linesync function instead of using the dataserver event. Let me attempt repro with that code
H
Harold Linden
Jamesp1989 Resident:
Hmm, yeah, using those potentially long-running synchronous functions right before a
string.match()
would be a problem. A lot of the ll.*()
functions technically put you over or right near the script time limit, but we allow it because the time it takes is our fault, not yours.Wrapping it in a coroutine wouldn't help, no matter what we can't yield once we're
inside
the call to string.match()
, and there are regular script time overrun checks inside the implementation of that function, so we have no option but to throw an error to get out of it.WolfGang Senizen
Harold Linden
The unable to yield error can at least be pcall'd so its not an unrecoverable error.
H
Harold Linden
I expect if you put an
ll.Sleep(0.00001)
right before that string.match()
it'd hack around the issue for now. That would make sure you're at the start of your script's timeslice when it comes back to call string.match()
.We'll likely implement a dynamically injected version of that ourselves before those calls.
H
Harold Linden
WolfGang Senizen It's not supposed to be recoverable, we just didn't fix that yet ;)
H
Harold Linden
I think we could help this a lot by
- trying to give a context-sensitive error when we're in a place that we can't yield. If we're inside string.match()or whatever, it'd be more helpful to give a warning about pattern complexity. If you're inside a metamethod that you can'tyieldinside (basically anything other than__call, we should just say so.) Ideally with a proper stacktrace.
- Make string.match()do yield checks beforehand, as right now multiple successivestring.match()es won't do them, and will just explode at you once you reach the "punishment" threshold for time slice overruns.
WolfGang Senizen
+1
I managed to reproduce thisI just slapped a random slightly large notecard in the prim called it
LootConfig
and saved the script.Object [script:New Script] Script run-time error
Unable to perform mandatory yield
Unable to perform mandatory yield