From 1620a92d54359a2607d43b3d324030a290a0b362 Mon Sep 17 00:00:00 2001 From: Takase <20792268+takase1121@users.noreply.github.com> Date: Sun, 22 Oct 2023 01:17:49 +0800 Subject: [PATCH] fix(dirmonitor): deadlock if error handler jumps somewhere else (#1647) * fix: deadlock if error handler jumps somewhere else * docs(dirmonitor): fix wrong data type in error callback * docs(dirmonitor): clarify coroutines and deadlocks * docs(dirmonitor): wording Co-authored-by: Guldoman --------- Co-authored-by: Guldoman --- data/core/dirwatch.lua | 4 ++++ docs/api/dirmonitor.lua | 6 +++++- src/api/dirmonitor.c | 22 +++++++++++++++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/data/core/dirwatch.lua b/data/core/dirwatch.lua index e3b6d61c..3b62fa24 100644 --- a/data/core/dirwatch.lua +++ b/data/core/dirwatch.lua @@ -91,6 +91,7 @@ end -- designed to be run inside a coroutine. function dirwatch:check(change_callback, scan_time, wait_time) local had_change = false + local last_error self.monitor:check(function(id) had_change = true if self.monitor:mode() == "single" then @@ -102,7 +103,10 @@ function dirwatch:check(change_callback, scan_time, wait_time) elseif self.reverse_watched[id] then change_callback(self.reverse_watched[id]) end + end, function(err) + last_error = err end) + if last_error ~= nil then error(last_error) end local start_time = system.get_time() for directory, old_modified in pairs(self.scanned) do if old_modified then diff --git a/docs/api/dirmonitor.lua b/docs/api/dirmonitor.lua index 439e0ec4..a12b61f1 100644 --- a/docs/api/dirmonitor.lua +++ b/docs/api/dirmonitor.lua @@ -45,10 +45,14 @@ function dirmonitor:unwatch(fd_or_path) end ---edited, removed or added. A file descriptor will be passed to the ---callback in "multiple" mode or a path in "single" mode. --- +---If an error occurred during the callback execution, the error callback will be called with the error object. +---This callback should not manipulate coroutines to avoid deadlocks. +--- ---@param callback dirmonitor.callback +---@param error_callback fun(error: any): nil --- ---@return boolean? changes True when changes were detected. -function dirmonitor:check(callback) end +function dirmonitor:check(callback, error_callback) end --- ---Get the working mode for the current file system monitoring backend. diff --git a/src/api/dirmonitor.c b/src/api/dirmonitor.c index 1bccfd13..320235a0 100644 --- a/src/api/dirmonitor.c +++ b/src/api/dirmonitor.c @@ -1,4 +1,5 @@ #include "api.h" +#include "lua.h" #include #include #include @@ -25,13 +26,16 @@ int get_mode_dirmonitor(); static int f_check_dir_callback(int watch_id, const char* path, void* L) { - lua_pushvalue(L, -1); + // using absolute indices from f_dirmonitor_check (2: callback, 3: error_callback) + lua_pushvalue(L, 2); if (path) lua_pushlstring(L, path, watch_id); else lua_pushnumber(L, watch_id); - lua_call(L, 1, 1); - int result = lua_toboolean(L, -1); + + int result = 0; + if (lua_pcall(L, 1, 1, 3) == LUA_OK) + result = lua_toboolean(L, -1); lua_pop(L, 1); return !result; } @@ -95,8 +99,20 @@ static int f_dirmonitor_unwatch(lua_State *L) { } +static int f_noop(lua_State *L) { return 0; } + + static int f_dirmonitor_check(lua_State* L) { struct dirmonitor* monitor = luaL_checkudata(L, 1, API_TYPE_DIRMONITOR); + luaL_checktype(L, 2, LUA_TFUNCTION); + if (!lua_isnoneornil(L, 3)) { + luaL_checktype(L, 3, LUA_TFUNCTION); + } else { + lua_settop(L, 2); + lua_pushcfunction(L, f_noop); + } + lua_settop(L, 3); + SDL_LockMutex(monitor->mutex); if (monitor->length < 0) lua_pushnil(L);