fix: avoid iterating over a changing table in run_threads (#1794)
* fix: avoid iterating over a changing table in `run_threads`
This is done to avoid iterating over a table that can change in the meantime.
More precisely the issue appears if a thread is removed from the table, we yield early from `run_threads` because we reached the end of the frame, and a new thread is added before the next iteration.
For example:
```lua
local lost_time = false
core.add_thread(function()
-- force early yield
local t0 = system.get_time()
while system.get_time() - t0 < .1 do end
lost_time = true
end, "a")
local step = core.step
function core.step()
if lost_time then
-- add a new thread while run_threads hasn't finished iterating
core.add_thread(function()end, "a1")
lost_time = false
end
return step()
end
```
would crash with `invalid key to 'next'`.
* fix: only run coroutine if it wasn't removed
* fix: don't handle `core.threads` table as an array
This caused some entries to be skipped or even removed erroneously.
This commit is contained in:
+18
-8
@@ -1092,8 +1092,14 @@ function core.show_title_bar(show)
|
||||
end
|
||||
|
||||
|
||||
local thread_counter = 0
|
||||
function core.add_thread(f, weak_ref, ...)
|
||||
local key = weak_ref or #core.threads + 1
|
||||
local key = weak_ref
|
||||
if not key then
|
||||
thread_counter = thread_counter + 1
|
||||
key = thread_counter
|
||||
end
|
||||
assert(core.threads[key] == nil, "Duplicate thread reference")
|
||||
local args = {...}
|
||||
local fn = function() return core.try(f, table.unpack(args)) end
|
||||
core.threads[key] = { cr = coroutine.create(fn), wake = 0 }
|
||||
@@ -1356,16 +1362,20 @@ local run_threads = coroutine.wrap(function()
|
||||
local max_time = 1 / config.fps - 0.004
|
||||
local minimal_time_to_wake = math.huge
|
||||
|
||||
local threads = {}
|
||||
-- We modify core.threads while iterating, both by removing dead threads,
|
||||
-- and by potentially adding more threads while we yielded early,
|
||||
-- so we need to extract the threads list and iterate over that instead.
|
||||
for k, thread in pairs(core.threads) do
|
||||
-- run thread
|
||||
if thread.wake < system.get_time() then
|
||||
threads[k] = thread
|
||||
end
|
||||
|
||||
for k, thread in pairs(threads) do
|
||||
-- Run thread if it wasn't deleted externally and it's time to resume it
|
||||
if core.threads[k] and thread.wake < system.get_time() then
|
||||
local _, wait = assert(coroutine.resume(thread.cr))
|
||||
if coroutine.status(thread.cr) == "dead" then
|
||||
if type(k) == "number" then
|
||||
table.remove(core.threads, k)
|
||||
else
|
||||
core.threads[k] = nil
|
||||
end
|
||||
core.threads[k] = nil
|
||||
elseif wait then
|
||||
thread.wake = system.get_time() + wait
|
||||
minimal_time_to_wake = math.min(minimal_time_to_wake, wait)
|
||||
|
||||
Reference in New Issue
Block a user