AutoSuspend: Don't send LeaveStandby events from a zombie plugin instance (#9124)

Long story short: the LeaveStandby event is sent via `tickAfterNext`, so if we tear down the plugin right after calling it (in this case, that means that the very input event that wakes the device up from suspend is one that kills ReaderUI or FileManager), what's in UIManager's task queue isn't the actual function, but the anonymous nextTick wrapper constructed by `tickAfterNext` (c.f., 
https://github.com/koreader/koreader/issues/9112#issuecomment-1133999385).

Tweak `UIManager:tickAfterNext` to return a reference to said wrapper, so that we can store it and unschedule that one, too, in `AutoSuspend:onCloseWidget`.

Fix #9112 (many thanks to [@boredhominid](https://github.com/boredhominid) for his help in finding a repro for this ;)).
Re: #8638, as the extra debugging facilities (i.e., ebb81b9845) added during testing might help pinpoint the root issue for that one, too.

Also includes a minor simplification to `UIManager:_checkTasks`, and various other task queue related codepaths (e.g., `WakeupMgr`) ;).
reviewable/pr9076/r33^2
NiLuJe 2 years ago committed by GitHub
parent 46f09b9e6c
commit ae5b2d68ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -996,7 +996,7 @@ function ReaderDictionary:showDict(word, results, boxes, link)
if not results.lookup_cancelled and self._lookup_start_time
and (time.now() - self._lookup_start_time) > self.quick_dismiss_before_delay then
-- If the search took more than a few seconds to be done, discard
-- queued and coming up input events to avoid a voluntary dismissal
-- queued and upcoming input events to avoid a voluntary dismissal
-- (because the user felt the result would not come) to kill the
-- result that finally came and is about to be displayed
Input:inhibitInputUntil(true)

@ -88,7 +88,7 @@ with anonymous functions.
@treturn bool (true if one or more tasks were removed; false otherwise; nil if the task queue is empty).
--]]
function WakeupMgr:removeTasks(epoch, callback)
if #self._task_queue < 1 then return end
if #self._task_queue == 0 then return end
local removed = false
local reschedule = false
@ -147,7 +147,7 @@ If necessary, the next upcoming task (if any) is scheduled on exit.
@treturn bool (true if we were truly woken up by the scheduled wakeup; false otherwise; nil if there weren't any tasks scheduled).
--]]
function WakeupMgr:wakeupAction(proximity)
if #self._task_queue > 0 then
if self._task_queue[1] then
local task = self._task_queue[1]
if self:validateWakeupAlarmByProximity(task.epoch, proximity) then
task.callback()

@ -521,13 +521,13 @@ function UIManager:close(widget, refreshtype, refreshregion, refreshdither)
end
end
-- schedule an execution task, task queue is in ascendant order
-- Schedule an execution task; task queue is in ascending order
function UIManager:schedule(sched_time, action, ...)
local p, s, e = 1, 1, #self._task_queue
if e ~= 0 then
-- do a binary insert
-- Do a binary insert.
repeat
p = math.floor((e + s) / 2) -- Not necessary to use (s + (e -s) / 2) here!
p = bit.rshift(e + s, 1) -- Not necessary to use (s + (e -s) / 2) here!
local p_time = self._task_queue[p].time
if sched_time > p_time then
if s == e then
@ -544,12 +544,13 @@ function UIManager:schedule(sched_time, action, ...)
end
e = p
else
-- for fairness, it's better to make p+1 is strictly less than
-- p might want to revisit here in the future
-- For fairness, it's better to make sure p+1 is strictly less than p.
-- Might want to revisit that in the future.
break
end
until e < s
end
table.insert(self._task_queue, p, {
time = sched_time,
action = action,
@ -602,6 +603,9 @@ Useful to run UI callbacks ASAP without skipping repaints.
@func action reference to the task to be scheduled (may be anonymous)
@param ... optional arguments passed to action
@return A reference to the initial nextTick wrapper function,
necessary if the caller wants to unschedule action *before* it actually gets inserted in the task queue by nextTick.
@see nextTick
]]
function UIManager:tickAfterNext(action, ...)
@ -609,7 +613,14 @@ function UIManager:tickAfterNext(action, ...)
-- c.f., http://lua-users.org/wiki/VarargTheSecondClassCitizen
local n = select('#', ...)
local va = {...}
return self:nextTick(function() self:nextTick(action, unpack(va, 1, n)) end)
-- We need to keep a reference to this anonymous function, as it is *NOT* quite `action` yet,
-- and the caller might want to unschedule it early...
local action_wrapper = function()
self:nextTick(action, unpack(va, 1, n))
end
self:nextTick(action_wrapper)
return action_wrapper
end
--[[
-- NOTE: This appears to work *nearly* just as well, but does sometimes go too fast (might depend on kernel HZ & NO_HZ settings?)
@ -1142,7 +1153,7 @@ end
--]]
function UIManager:getNextTaskTime()
if #self._task_queue > 0 then
if self._task_queue[1] then
return self._task_queue[1].time - time:now()
else
return nil
@ -1153,25 +1164,21 @@ function UIManager:_checkTasks()
self._now = time.now()
local wait_until = nil
-- task.action may schedule other events
-- Tasks due for execution might themselves schedule more tasks (that might also be immediately due for execution ;)).
-- Flipping this switch ensures we'll consume all such tasks *before* yielding to input polling.
self._task_queue_dirty = false
while true do
if #self._task_queue == 0 then
-- Nothing to do!
break
end
local next_task = self._task_queue[1]
local task_time = next_task.time or 0
while self._task_queue[1] do
local task_time = self._task_queue[1].time
if task_time <= self._now then
-- remove from table
-- Pop the upcoming task, as it is due for execution...
local task = table.remove(self._task_queue, 1)
-- task is pending to be executed right now. do it.
-- NOTE: be careful that task.action() might modify
-- _task_queue here. So need to avoid race condition
-- ...so do it now.
-- NOTE: Said task's action might modify _task_queue.
-- To avoid race conditions and catch new upcoming tasks during this call,
-- we repeatedly check the head of the queue (c.f., #1758).
task.action(unpack(task.args, 1, task.argc))
else
-- queue is sorted in ascendant order, safe to assume all items
-- are future tasks for now
-- As the queue is sorted in ascending order, it's safe to assume all items are currently future tasks.
wait_until = task_time
break
end

@ -186,7 +186,7 @@ function ConfirmBox:onShow()
return "ui", self[1][1].dimen
end)
if self.flush_events_on_show then
-- Discard queued and coming up input events to avoid accidental dismissal
-- Discard queued and upcoming input events to avoid accidental dismissal
Input:inhibitInputUntil(true)
end
end

@ -234,7 +234,7 @@ function InfoMessage:onShow()
return "ui", self[1][1].dimen
end)
if self.flush_events_on_show then
-- Discard queued and coming up input events to avoid accidental dismissal
-- Discard queued and upcoming input events to avoid accidental dismissal
Input:inhibitInputUntil(true)
end
-- schedule us to close ourself if timeout provided

@ -33,11 +33,12 @@ local AutoSuspend = WidgetContainer:new{
auto_suspend_timeout_seconds = default_auto_suspend_timeout_seconds,
auto_standby_timeout_seconds = default_auto_standby_timeout_seconds,
last_action_time = 0,
is_standby_scheduled = false,
is_standby_scheduled = nil,
task = nil,
standby_task = nil,
leave_standby_task = nil,
going_to_suspend = false,
wrapped_leave_standby_task = nil,
going_to_suspend = nil,
}
function AutoSuspend:_enabledStandby()
@ -135,6 +136,10 @@ function AutoSuspend:init()
if Device:isPocketBook() and not Device:canSuspend() then return end
-- We only want those to exist as *instance* members
self.is_standby_scheduled = false
self.going_to_suspend = false
UIManager.event_hook:registerWidget("InputEvent", self)
-- We need an instance-specific function reference to schedule, because in some rare cases,
-- we may instantiate a new plugin instance *before* tearing down the old one.
@ -199,7 +204,14 @@ function AutoSuspend:_unschedule_standby()
end
-- Make sure we don't trigger a ghost LeaveStandby event...
if self.wrapped_leave_standby_task then
logger.dbg("AutoSuspend: unschedule leave standby task wrapper")
UIManager:unschedule(self.wrapped_leave_standby_task)
self.wrapped_leave_standby_task = nil
end
if self.leave_standby_task then
logger.dbg("AutoSuspend: unschedule leave standby task")
UIManager:unschedule(self.leave_standby_task)
end
end
@ -326,6 +338,8 @@ end
function AutoSuspend:onLeaveStandby()
logger.dbg("AutoSuspend: onLeaveStandby")
-- If the Event got through, tickAfterNext did its thing, clear the reference to the initial nextTick wrapper...
self.wrapped_leave_standby_task = nil
-- Unschedule suspend and shutdown, as the realtime clock has ticked
self:_unschedule()
-- Reschedule suspend and shutdown (we'll recompute the delay based on the last user input, *not* the current time).
@ -587,7 +601,7 @@ function AutoSuspend:AllowStandbyHandler()
-- (in case we were woken up by user input, as opposed to an rtc wake alarm)!
-- (This ensures we'll use an up to date last_action_time, and that it only ever gets updated from *user* input).
-- NOTE: UIManager consumes scheduled tasks before input events, which is why we can't use nextTick.
UIManager:tickAfterNext(self.leave_standby_task)
self.wrapped_leave_standby_task = UIManager:tickAfterNext(self.leave_standby_task)
end
end

Loading…
Cancel
Save