From 830a737381bfb15beb1ab45f14ae658967163e8e Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Tue, 26 Jun 2018 18:14:29 +0200 Subject: [PATCH 1/4] resty/concurrent/timer_task: adapt to use new GC module --- gateway/src/resty/concurrent/timer_task.lua | 23 +++++++++------------ 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/gateway/src/resty/concurrent/timer_task.lua b/gateway/src/resty/concurrent/timer_task.lua index 3081275bd..33c1f1135 100644 --- a/gateway/src/resty/concurrent/timer_task.lua +++ b/gateway/src/resty/concurrent/timer_task.lua @@ -1,7 +1,6 @@ +local GC = require 'apicast.gc' + local uuid = require 'resty.jit-uuid' -local setmetatable = setmetatable -local getmetatable = getmetatable -local newproxy = newproxy local unpack = unpack local _M = {} @@ -26,17 +25,15 @@ local function generate_id() return uuid.generate_v4() end -local function mt(id) - -- Using 'newproxy' is needed to be able to define __gc(). - -- With __gc we can make sure that the task will stop scheduling more work - -- after it has been garbage collected. - local proxy = newproxy(true) - local res_mt = getmetatable(proxy) - res_mt.__gc = function() _M.unregister_task(id) end - res_mt.__index = _M - return res_mt +local function gc(self) + _M.unregister_task(self.id) end +local mt = { + __gc = gc, + __index = _M +} + --- Initialize a TimerTask. -- @tparam function task The function to be run periodically -- @tparam[opt] table opts @@ -47,7 +44,7 @@ function _M.new(task, opts) local id = generate_id() - local self = setmetatable({}, mt(id)) + local self = GC.set_metatable_gc({}, mt) self.task = task self.args = options.args self.interval = options.interval or default_interval_seconds From 7aae6d7c13ba0218b075ae1e2de13c726c5cf777 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Tue, 26 Jun 2018 18:14:43 +0200 Subject: [PATCH 2/4] spec/resty/concurrent/timer_task: use spies instead of stubs TimerTask is now built calling GC.set_metatable_gc. That method returns an object for which type() returns "userdata". Luassert only allows creating stubs on objects for which type() returns "table". --- spec/resty/concurrent/timer_task_spec.lua | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/resty/concurrent/timer_task_spec.lua b/spec/resty/concurrent/timer_task_spec.lua index 7cb36416c..2e217f830 100644 --- a/spec/resty/concurrent/timer_task_spec.lua +++ b/spec/resty/concurrent/timer_task_spec.lua @@ -85,11 +85,11 @@ describe('TimerTask', function() describe('when the task is active', function() it('runs the task', function() local timer_task = TimerTask.new(func, { args = args, interval = interval }) - local func_stub = stub(timer_task, 'task') + local func_spy = spy.on(timer_task, 'task') timer_task:execute(true) - assert.stub(func_stub).was_called_with(unpack(args)) + assert.spy(func_spy).was_called_with(unpack(args)) end) it('schedules the next one', function() @@ -104,12 +104,12 @@ describe('TimerTask', function() describe('when the task is not active', function() it('does not run the task', function() local timer_task = TimerTask.new(func, { args = args, interval = interval }) - local func_stub = stub(timer_task, 'task') + local func_spy = spy.on(timer_task, 'task') timer_task:cancel() timer_task:execute(true) - assert.stub(func_stub).was_not_called() + assert.spy(func_spy).was_not_called() end) it('does not schedule another task', function() @@ -125,12 +125,12 @@ describe('TimerTask', function() describe('when the option to wait an interval instead of running now is passed', function() it('does not run the task inmediately', function() local timer_task = TimerTask.new(func, { args = args, interval = interval }) - local func_stub = stub(timer_task, 'task') + local func_spy = spy.on(timer_task, 'task') timer_task:execute(false) -- It will be called in 'interval' seconds, but not now - assert.stub(func_stub).was_not_called() + assert.spy(func_spy).was_not_called() end) it('schedules the next one', function() From adbac7653ae47b3ae3c84cdf445eb7279e555e0c Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Mon, 25 Jun 2018 19:09:13 +0200 Subject: [PATCH 3/4] resty/concurrent/timer_task: avoid references to self So instances can be garbage-collected --- gateway/src/resty/concurrent/timer_task.lua | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/gateway/src/resty/concurrent/timer_task.lua b/gateway/src/resty/concurrent/timer_task.lua index 33c1f1135..f1c126e7f 100644 --- a/gateway/src/resty/concurrent/timer_task.lua +++ b/gateway/src/resty/concurrent/timer_task.lua @@ -57,25 +57,25 @@ end local run_periodic, schedule_next, timer_execute -run_periodic = function(self, run_now) - if not _M.task_is_active(self.id) then return end +run_periodic = function(run_now, id, func, args, interval) + if not _M.task_is_active(id) then return end if run_now then - self.task(unpack(self.args)) + func(unpack(args)) end - schedule_next(self) + schedule_next(interval, id, func, args, interval) end -- Note: ngx.timer.at always sends "premature" as the first param. -- "premature" is boolean value indicating whether it is a premature timer -- expiration. -timer_execute = function(_, self) - run_periodic(self, true) +timer_execute = function(_, id, func, args, interval) + run_periodic(true, id, func, args, interval) end -schedule_next = function(self) - local ok, err = ngx.timer.at(self.interval, timer_execute, self) +schedule_next = function(id, func, args, interval) + local ok, err = ngx.timer.at(interval, timer_execute, id, func, args, interval) if not ok then ngx.log(ngx.ERR, "failed to schedule timer task: ", err) @@ -86,7 +86,7 @@ end -- @tparam[opt] run_now boolean True to run the task immediately or False to -- wait 'interval' seconds. (Defaults to false) function _M:execute(run_now) - run_periodic(self, run_now or false) + run_periodic(run_now or false, self.id, self.task, self.args, self.interval) end function _M:cancel() From 998ed2677430971b757cf680149c9cebacadaed0 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 27 Jun 2018 11:23:39 +0200 Subject: [PATCH 4/4] CHANGELOG: add TimerTask improvements --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d14c1c21..a4c7c8c63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - ENV variables to make APIcast listen on HTTPS port [PR #622](https://github.com/3scale/apicast/pull/622) - New `ssl_certificate` phase allows policies to provide certificate to terminate HTTPS connection [PR #622](https://github.com/3scale/apicast/pull/622). - Configurable `auth_type` for the token introspection policy [PR #755](https://github.com/3scale/apicast/pull/755) -- `TimerTask` module to execute recurrent tasks that can be cancelled [PR #782](https://github.com/3scale/apicast/pull/782), [#784](https://github.com/3scale/apicast/pull/784) +- `TimerTask` module to execute recurrent tasks that can be cancelled [PR #782](https://github.com/3scale/apicast/pull/782), [PR #784](https://github.com/3scale/apicast/pull/784), [PR #791](https://github.com/3scale/apicast/pull/791) - `GC` module that implements a workaround to be able to define `__gc` on tables [PR #790](https://github.com/3scale/apicast/pull/790) ### Changed