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:
Guldoman 2024-06-05 14:58:19 +02:00 committed by GitHub
parent e0a7bdbcba
commit 4d649ba92d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 18 additions and 8 deletions

View File

@ -1127,8 +1127,14 @@ function core.show_title_bar(show)
end end
local thread_counter = 0
function core.add_thread(f, weak_ref, ...) 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 args = {...}
local fn = function() return core.try(f, table.unpack(args)) end local fn = function() return core.try(f, table.unpack(args)) end
core.threads[key] = { cr = coroutine.create(fn), wake = 0 } core.threads[key] = { cr = coroutine.create(fn), wake = 0 }
@ -1402,16 +1408,20 @@ local run_threads = coroutine.wrap(function()
local max_time = 1 / config.fps - 0.004 local max_time = 1 / config.fps - 0.004
local minimal_time_to_wake = math.huge 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 for k, thread in pairs(core.threads) do
-- run thread threads[k] = thread
if thread.wake < system.get_time() then 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)) local _, wait = assert(coroutine.resume(thread.cr))
if coroutine.status(thread.cr) == "dead" then if coroutine.status(thread.cr) == "dead" then
if type(k) == "number" then
table.remove(core.threads, k)
else
core.threads[k] = nil core.threads[k] = nil
end
else else
wait = wait or (1/30) wait = wait or (1/30)
thread.wake = system.get_time() + wait thread.wake = system.get_time() + wait