Skip to content

Commit

Permalink
Merge pull request #688 from 3scale/policy-gc
Browse files Browse the repository at this point in the history
[policy] run __gc metamethod when policies are garbage collected
  • Loading branch information
davidor authored Jun 29, 2018
2 parents 7fc66dc + 6925516 commit 0fa2ed4
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- 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), [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)
- Policies can define `__gc` metamethod that gets called when they are garbage collected to do cleanup [PR #688](https://github.com/3scale/apicast/pull/688)

### Changed

Expand Down
6 changes: 5 additions & 1 deletion docker-compose.benchmark.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
version: '2.1'
version: '2.2'
services:
apicast:
image: quay.io/3scale/apicast:${IMAGE_TAG:-master}
command: bin/apicast -c /tmp/apicast/echo.json -b
volumes:
- ${CIRCLE_WORKING_DIRECTORY:-.}/examples/configuration/:/tmp/apicast/:ro
environment:
APICAST_WORKERS: 1
cpuset: "0"
cpu_count: 1
wrk:
image: skandyla/wrk
environment:
Expand Down
4 changes: 3 additions & 1 deletion gateway/src/apicast/configuration_loader.lua
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ function _M.configure(configuration, contents)
end

if config then
return configuration:store(config, ttl())
configuration:store(config, ttl())
collectgarbage()
return config
end
end

Expand Down
38 changes: 35 additions & 3 deletions gateway/src/apicast/gc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,26 @@
-- works in "userdata". This module introduces a workaround to make it work
-- with tables.

local tab_clone = require "table.clone"

local rawgetmetatable = debug.getmetatable
local getmetatable = getmetatable
local setmetatable = setmetatable
local newproxy = newproxy
local ipairs = ipairs
local pairs = pairs
local pcall = pcall
local table = table
local remove = table.remove
local unpack = unpack
local error = error
local tostring = tostring

local _M = {}

local function original_table(proxy)
return rawgetmetatable(proxy).__table
local mt = rawgetmetatable(proxy)

return mt and mt.__table
end

local function __gc(proxy)
Expand All @@ -37,7 +41,7 @@ local function __call(proxy, ...)
-- Try to run __call() and if it's not possible, try to run it in a way that
-- it returns a meaningful error.
local ret = { pcall(t, ...) }
local ok = table.remove(ret, 1)
local ok = remove(ret, 1)

if ok then
return unpack(ret)
Expand Down Expand Up @@ -91,4 +95,32 @@ function _M.set_metatable_gc(t, metatable)
return proxy
end

local delegate_mt = {
__gc = function(self)
local mt = getmetatable(self.table)

if mt and mt.__gc then return mt.__gc(self.table) end
end
}

--- Creates new object that when GC'd will call __gc metamethod on a given table.
--- @tparam table t a table
function _M.delegate(t)
return _M.set_metatable_gc({ table = t }, delegate_mt)
end

--- Clones given metatable so it is tied to the life of given object.
--- Please not that it first clones the metatable before assigning.
--- @tparam table t a table
--- @tparam table metatable a metatable
function _M.setmetatable_gc_clone(t, metatable)
local copy = tab_clone(metatable)

copy.__table = t
copy.__gc_helper = _M.delegate(t)
copy.__metatable = metatable

return setmetatable(t, copy)
end

return _M
11 changes: 8 additions & 3 deletions gateway/src/apicast/policy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ local PHASES = {
'ssl_certificate',
}

local setmetatable = setmetatable
local GC = require('apicast.gc')
local setmetatable_gc_clone = GC.setmetatable_gc_clone
local ipairs = ipairs
local format = string.format

Expand All @@ -37,22 +38,26 @@ end
-- @tparam string name Name of the new policy.
-- @tparam string version Version of the new policy. Default value is 0.0
-- @treturn policy New policy
-- @treturn table New policy metatable.
function _M.new(name, version)
local policy = {
_NAME = name,
_VERSION = version or '0.0',
}

local mt = { __index = policy, __tostring = __tostring, policy = policy }

function policy.new()
return setmetatable({}, mt)
local p = setmetatable_gc_clone({}, mt)

return p
end

for _, phase in _M.phases() do
policy[phase] = noop
end

return setmetatable(policy, { __tostring = __tostring, __eq = __eq })
return setmetatable(policy, { __tostring = __tostring, __eq = __eq }), mt
end

function _M.phases()
Expand Down
2 changes: 1 addition & 1 deletion gateway/src/apicast/policy/echo/echo.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
-- Also can interrupt the execution and skip the current phase or
-- the whole processing of the request.

local _M = require('apicast.policy').new('Echo Policy')
local _M = require('apicast.policy').new('Echo Policy')
local cjson = require('cjson')

local tonumber = tonumber
Expand Down
8 changes: 5 additions & 3 deletions script/s2i
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ DOCKER_REPO_ROOT=/opt/app-root/src
source_volume() {
volume=$(docker volume create)
container=$(docker create --user root --volume "${volume}:${DOCKER_REPO_ROOT}/cache" "${IMAGE_NAME}" sh -c 'chgrp -fR root . && chmod -fR g+w .')
docker cp "$PWD/local" "${container}:${DOCKER_REPO_ROOT}/cache/perl5" 2>/dev/null || true
docker cp "$PWD/lua_modules" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
docker cp "$PWD/vendor" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
if [ -n "${CI:-}" ]; then
docker cp "$PWD/local" "${container}:${DOCKER_REPO_ROOT}/cache/perl5" 2>/dev/null || true
docker cp "$PWD/lua_modules" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
docker cp "$PWD/vendor" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
fi
docker start "${container}" >/dev/null
docker wait "${container}" >/dev/null
docker logs "${container}" >&2
Expand Down
17 changes: 17 additions & 0 deletions spec/policy_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,21 @@ describe('policy', function()
assert.same(phases, res)
end)
end)

describe('garbage collection', function()
it('runs __gc metamethod when a policy instance is garbage-collected', function()
local MyPolicy, mt = policy.new('my_policy', '1.0')
local property
mt.__gc = spy.new(function(instance) property = instance.someproperty end)
local p = MyPolicy.new()
p.someproperty = 'foobar'
p = nil
assert.is_nil(p)

collectgarbage()

assert.spy(mt.__gc).was_called(1)
assert.equal('foobar', property)
end)
end)
end)

0 comments on commit 0fa2ed4

Please sign in to comment.