From 5c7e511b44a3b8f5e4f5190d8115fa03f2ce686d Mon Sep 17 00:00:00 2001
From: Thijs Schreijer <thijs@thijsschreijer.nl>
Date: Tue, 21 Mar 2017 18:47:02 +0100
Subject: [PATCH] fix(cache) avoid deadlocks upon callback errors

Main cause is exiting early on error conditions, when the lock has not
yet been released. It occurs both in the `get_or_set` method itself as
well as in the callbacks all over the code base.

Fix #2186
---
 kong/core/plugins_iterator.lua          |  7 ++-
 kong/plugins/acl/handler.lua            |  8 +--
 kong/plugins/basic-auth/access.lua      | 20 ++++++--
 kong/plugins/hmac-auth/access.lua       | 23 ++++++---
 kong/plugins/jwt/handler.lua            | 19 ++++++--
 kong/plugins/key-auth/handler.lua       | 21 +++++---
 kong/plugins/ldap-auth/access.lua       | 15 ++++--
 kong/plugins/oauth2/access.lua          | 38 ++++++++++-----
 kong/tools/database_cache.lua           | 65 ++++++++++++++++---------
 kong/tools/utils.lua                    |  7 +++
 spec/01-unit/04-utils_spec.lua          | 28 +++++++++++
 spec/01-unit/11-database_cache_spec.lua | 27 ++++++++++
 12 files changed, 213 insertions(+), 65 deletions(-)

diff --git a/kong/core/plugins_iterator.lua b/kong/core/plugins_iterator.lua
index 045087cf68f..9d81f3f4b7c 100644
--- a/kong/core/plugins_iterator.lua
+++ b/kong/core/plugins_iterator.lua
@@ -14,7 +14,7 @@ local function load_plugin_into_memory(api_id, consumer_id, plugin_name)
     name = plugin_name
   }
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
 
   if #rows > 0 then
@@ -37,8 +37,11 @@ end
 -- @treturn table Plugin retrieved from the cache or database.
 local function load_plugin_configuration(api_id, consumer_id, plugin_name)
   local cache_key = cache.plugin_key(plugin_name, api_id, consumer_id)
-  local plugin = cache.get_or_set(cache_key, nil, load_plugin_into_memory,
+  local plugin, err = cache.get_or_set(cache_key, nil, load_plugin_into_memory,
                                   api_id, consumer_id, plugin_name)
+  if err then
+    responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
   if plugin ~= nil and plugin.enabled then
     return plugin.config or {}
   end
diff --git a/kong/plugins/acl/handler.lua b/kong/plugins/acl/handler.lua
index b91b78aef94..af35c3b5232 100644
--- a/kong/plugins/acl/handler.lua
+++ b/kong/plugins/acl/handler.lua
@@ -21,7 +21,7 @@ end
 local function load_acls_into_memory(consumer_id)
   local results, err = singletons.dao.acls:find_all {consumer_id = consumer_id}
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return results
 end
@@ -37,9 +37,11 @@ function ACLHandler:access(conf)
   end
 
   -- Retrieve ACL
-  local acls = cache.get_or_set(cache.acls_key(consumer_id), nil,
+  local acls, err = cache.get_or_set(cache.acls_key(consumer_id), nil,
                                 load_acls_into_memory, consumer_id)
-
+  if err then
+    responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
   if not acls then acls = {} end
 
   local block
diff --git a/kong/plugins/basic-auth/access.lua b/kong/plugins/basic-auth/access.lua
index a49b4b70298..b6c2d26821a 100644
--- a/kong/plugins/basic-auth/access.lua
+++ b/kong/plugins/basic-auth/access.lua
@@ -69,7 +69,7 @@ end
 local function load_credential_into_memory(username)
   local credentials, err = singletons.dao.basicauth_credentials:find_all {username = username}
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return credentials[1]
 end
@@ -77,8 +77,12 @@ end
 local function load_credential_from_db(username)
   if not username then return end
   
-  return cache.get_or_set(cache.basicauth_credential_key(username),
+  local credential, err = cache.get_or_set(cache.basicauth_credential_key(username),
                           nil, load_credential_into_memory, username)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
+  return credential
 end
 
 local function load_consumer_into_memory(consumer_id, anonymous)
@@ -87,7 +91,7 @@ local function load_consumer_into_memory(consumer_id, anonymous)
     if anonymous and not err then
       err = 'anonymous consumer "'..consumer_id..'" not found'
     end
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return result
 end
@@ -132,8 +136,11 @@ local function do_authentication(conf)
   end
 
   -- Retrieve consumer
-  local consumer = cache.get_or_set(cache.consumer_key(credential.consumer_id),
+  local consumer, err = cache.get_or_set(cache.consumer_key(credential.consumer_id),
                    nil, load_consumer_into_memory, credential.consumer_id, false)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
 
   set_consumer(consumer, credential)
 
@@ -145,8 +152,11 @@ function _M.execute(conf)
   if not ok then
     if conf.anonymous ~= "" then
       -- get anonymous user
-      local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
+      local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
                        nil, load_consumer_into_memory, conf.anonymous, true)
+      if err then
+        return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+      end
       set_consumer(consumer, nil)
     else
       return responses.send(err.status, err.message)
diff --git a/kong/plugins/hmac-auth/access.lua b/kong/plugins/hmac-auth/access.lua
index 343a734c830..6d7a81002ce 100644
--- a/kong/plugins/hmac-auth/access.lua
+++ b/kong/plugins/hmac-auth/access.lua
@@ -106,17 +106,22 @@ end
 local function load_credential_into_memory(username)
   local keys, err = singletons.dao.hmacauth_credentials:find_all { username = username }
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return keys[1]
 end
 
 local function load_credential(username)
-  local credential
+  local credential, err
   if username then
-    credential = cache.get_or_set(cache.hmacauth_credential_key(username),
+    credential, err = cache.get_or_set(cache.hmacauth_credential_key(username),
                                   nil, load_credential_into_memory, username)
   end
+
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
+
   return credential
 end
 
@@ -144,7 +149,7 @@ local function load_consumer_into_memory(consumer_id, anonymous)
     if anonymous and not err then
       err = 'anonymous consumer "'..consumer_id..'" not found'
     end
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return result
 end
@@ -197,8 +202,11 @@ local function do_authentication(conf)
   end
 
   -- Retrieve consumer
-  local consumer = cache.get_or_set(cache.consumer_key(credential.consumer_id),
+  local consumer, err = cache.get_or_set(cache.consumer_key(credential.consumer_id),
                    nil, load_consumer_into_memory, credential.consumer_id)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
 
   set_consumer(consumer, credential)
 
@@ -210,8 +218,11 @@ function _M.execute(conf)
   if not ok then
     if conf.anonymous ~= "" then
       -- get anonymous user
-      local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
+      local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
                        nil, load_consumer_into_memory, conf.anonymous, true)
+      if err then
+        return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+      end
       set_consumer(consumer, nil)
     else
       return responses.send(err.status, err.message)
diff --git a/kong/plugins/jwt/handler.lua b/kong/plugins/jwt/handler.lua
index 86dbf596a82..443fd76d8a3 100644
--- a/kong/plugins/jwt/handler.lua
+++ b/kong/plugins/jwt/handler.lua
@@ -53,7 +53,7 @@ end
 local function load_credential(jwt_secret_key)
   local rows, err = singletons.dao.jwt_secrets:find_all {key = jwt_secret_key}
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR()
+    return nil, err
   end
   return rows[1]
 end
@@ -64,7 +64,7 @@ local function load_consumer(consumer_id, anonymous)
     if anonymous and not err then
       err = 'anonymous consumer "'..consumer_id..'" not found'
     end
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return result
 end
@@ -114,8 +114,11 @@ local function do_authentication(conf)
   end
 
   -- Retrieve the secret
-  local jwt_secret = cache.get_or_set(cache.jwtauth_credential_key(jwt_secret_key),
+  local jwt_secret, err = cache.get_or_set(cache.jwtauth_credential_key(jwt_secret_key),
                                       nil, load_credential, jwt_secret_key)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
 
   if not jwt_secret then
     return false, {status = 403, message = "No credentials found for given '"..conf.key_claim_name.."'"}
@@ -149,8 +152,11 @@ local function do_authentication(conf)
   end
 
   -- Retrieve the consumer
-  local consumer = cache.get_or_set(cache.consumer_key(jwt_secret_key),
+  local consumer, err = cache.get_or_set(cache.consumer_key(jwt_secret_key),
                                     nil, load_consumer, jwt_secret.consumer_id)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
 
   -- However this should not happen
   if not consumer then
@@ -169,8 +175,11 @@ function JwtHandler:access(conf)
   if not ok then
     if conf.anonymous ~= "" then
       -- get anonymous user
-      local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
+      local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
                        nil, load_consumer, conf.anonymous, true)
+      if err then
+        return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+      end
       set_consumer(consumer, nil)
     else
       return responses.send(err.status, err.message)
diff --git a/kong/plugins/key-auth/handler.lua b/kong/plugins/key-auth/handler.lua
index 7d1d68a9452..a8469d1d493 100644
--- a/kong/plugins/key-auth/handler.lua
+++ b/kong/plugins/key-auth/handler.lua
@@ -26,7 +26,7 @@ local function load_credential(key)
     key = key
   }
   if not creds then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return creds[1]
 end
@@ -37,7 +37,7 @@ local function load_consumer(consumer_id, anonymous)
     if anonymous and not err then
       err = 'anonymous consumer "'..consumer_id..'" not found'
     end
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return result
 end
@@ -98,8 +98,11 @@ local function do_authentication(conf)
   end
 
   -- retrieve our consumer linked to this API key
-  local credential = cache.get_or_set(cache.keyauth_credential_key(key),
+  local credential, err = cache.get_or_set(cache.keyauth_credential_key(key),
                                       nil, load_credential, key)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
 
   -- no credential in DB, for this key, it is invalid, HTTP 403
   if not credential then
@@ -111,8 +114,11 @@ local function do_authentication(conf)
   -----------------------------------------
 
   -- retrieve the consumer linked to this API key, to set appropriate headers
-  local consumer = cache.get_or_set(cache.consumer_key(credential.consumer_id),
+  local consumer, err = cache.get_or_set(cache.consumer_key(credential.consumer_id),
                                     nil, load_consumer, credential.consumer_id)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
 
   set_consumer(consumer, credential)
 
@@ -126,8 +132,11 @@ function KeyAuthHandler:access(conf)
   if not ok then
     if conf.anonymous ~= "" then
       -- get anonymous user
-      local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
-                       nil, load_consumer, conf.anonymous, true)
+      local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
+                            nil, load_consumer, conf.anonymous, true)
+      if err then
+        responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+      end
       set_consumer(consumer, nil)
     else
       return responses.send(err.status, err.message)
diff --git a/kong/plugins/ldap-auth/access.lua b/kong/plugins/ldap-auth/access.lua
index 1479f7fd6de..e188a20329d 100644
--- a/kong/plugins/ldap-auth/access.lua
+++ b/kong/plugins/ldap-auth/access.lua
@@ -42,7 +42,7 @@ local function ldap_authenticate(given_username, given_password, conf)
   ok, err = sock:connect(conf.ldap_host, conf.ldap_port)
   if not ok then
     ngx_log(ngx_error, "[ldap-auth] failed to connect to "..conf.ldap_host..":"..tostring(conf.ldap_port)..": ", err)
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err, responses.status_codes.HTTP_INTERNAL_SERVER_ERROR
   end
 
   if conf.start_tls then
@@ -68,7 +68,8 @@ end
 local function load_credential(given_username, given_password, conf)
   ngx_log(ngx_debug, "[ldap-auth] authenticating user against LDAP server: "..conf.ldap_host..":"..conf.ldap_port)
 
-  local ok, err = ldap_authenticate(given_username, given_password, conf)
+  local ok, err, status = ldap_authenticate(given_username, given_password, conf)
+  if status ~= nil then return nil, err, status end
   if err ~= nil then ngx_log(ngx_error, err) end
   if not ok then
     return nil
@@ -82,8 +83,9 @@ local function authenticate(conf, given_credentials)
     return false
   end
 
-  local credential = cache.get_or_set(cache.ldap_credential_key(ngx.ctx.api.id, given_username), 
+  local credential, err, status = cache.get_or_set(cache.ldap_credential_key(ngx.ctx.api.id, given_username), 
       conf.cache_ttl, load_credential, given_username, given_password, conf)
+  if status then responses.send(status, err) end
 
   return credential and credential.password == given_password, credential
 end
@@ -94,7 +96,7 @@ local function load_consumer(consumer_id, anonymous)
     if anonymous and not err then
       err = 'anonymous consumer "'..consumer_id..'" not found'
     end
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return result
 end
@@ -151,8 +153,11 @@ function _M.execute(conf)
   if not ok then
     if conf.anonymous ~= "" then
       -- get anonymous user
-      local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
+      local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
                        nil, load_consumer, conf.anonymous, true)
+      if err then
+        responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+      end
       set_consumer(consumer, nil)
     else
       return responses.send(err.status, err.message)
diff --git a/kong/plugins/oauth2/access.lua b/kong/plugins/oauth2/access.lua
index 235c6da2048..98d96d274eb 100644
--- a/kong/plugins/oauth2/access.lua
+++ b/kong/plugins/oauth2/access.lua
@@ -74,16 +74,19 @@ end
 local function load_oauth2_credential_by_client_id_into_memory(client_id)
   local credentials, err = singletons.dao.oauth2_credentials:find_all {client_id = client_id}
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return credentials[1]
 end
 
 local function get_redirect_uri(client_id)
-  local client
+  local client, err
   if client_id then
-    client = cache.get_or_set(cache.oauth2_credential_key(client_id), nil,
+    client, err = cache.get_or_set(cache.oauth2_credential_key(client_id), nil,
                    load_oauth2_credential_by_client_id_into_memory, client_id)
+    if err then
+      return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    end
   end
   return client and client.redirect_uri or nil, client
 end
@@ -380,7 +383,7 @@ local function load_token_into_memory(conf, api, access_token)
   local credentials, err = singletons.dao.oauth2_tokens:find_all { api_id = api_id, access_token = access_token }
   local result
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   elseif #credentials > 0 then
     result = credentials[1]
   end
@@ -388,10 +391,13 @@ local function load_token_into_memory(conf, api, access_token)
 end
 
 local function retrieve_token(conf, access_token)
-  local token
+  local token, err
   if access_token then
-    token = cache.get_or_set(cache.oauth2_token_key(access_token), nil,
+    token, err = cache.get_or_set(cache.oauth2_token_key(access_token), nil,
                              load_token_into_memory, conf, ngx.ctx.api, access_token)
+    if err then
+      return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    end
   end
   return token
 end
@@ -439,7 +445,7 @@ end
 local function load_oauth2_credential_into_memory(credential_id)
   local result, err = singletons.dao.oauth2_credentials:find {id = credential_id}
   if err then
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return result
 end
@@ -450,7 +456,7 @@ local function load_consumer_into_memory(consumer_id, anonymous)
     if anonymous and not err then
       err = 'anonymous consumer "'..consumer_id..'" not found'
     end
-    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+    return nil, err
   end
   return result
 end
@@ -495,13 +501,20 @@ local function do_authentication(conf)
   end
 
   -- Retrieve the credential from the token
-  local credential = cache.get_or_set(cache.oauth2_credential_key(token.credential_id), 
+  local credential, err = cache.get_or_set(cache.oauth2_credential_key(token.credential_id), 
                         nil, load_oauth2_credential_into_memory, token.credential_id)
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
 
   -- Retrieve the consumer from the credential
-  local consumer = cache.get_or_set(cache.consumer_key(credential.consumer_id),
+  local consumer, err = cache.get_or_set(cache.consumer_key(credential.consumer_id),
     nil, load_consumer_into_memory, credential.consumer_id)
 
+  if err then
+    return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+  end
+
   set_consumer(consumer, credential, token)
 
   return true
@@ -535,8 +548,11 @@ function _M.execute(conf)
   if not ok then
     if conf.anonymous ~= "" then
       -- get anonymous user
-      local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
+      local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
                        nil, load_consumer_into_memory, conf.anonymous, true)
+      if err then
+        return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
+      end
       set_consumer(consumer, nil, nil)
     else
       return responses.send(err.status, err.message, err.headers)
diff --git a/kong/tools/database_cache.lua b/kong/tools/database_cache.lua
index 8317790687b..4bed0860265 100644
--- a/kong/tools/database_cache.lua
+++ b/kong/tools/database_cache.lua
@@ -1,9 +1,12 @@
+local utils = require "kong.tools.utils"
 local resty_lock = require "resty.lock"
 local json_encode = require("cjson.safe").encode
 local json_decode = require("cjson.safe").decode
 local cache = ngx.shared.cache
 local ngx_log = ngx.log
 local gettime = ngx.now
+local pack = utils.pack
+local unpack = utils.unpack
 
 local TTL_EXPIRE_KEY = "___expire_ttl"
 
@@ -27,6 +30,10 @@ local CACHE_KEYS = {
   TARGETS = "targets",
 }
 
+local function log(lvl, ...)
+  return ngx_log(lvl, "[db cache] ", ...)
+end
+
 local _M = {}
 
 -- Shared Dictionary
@@ -128,14 +135,17 @@ end
 -- **IMPORTANT:** the callback function may not exit the request early by e.g.
 -- sending a 404 response from the callback. The callback will be nested inside
 -- lock/unlock calls, and hence it MUST return or the lock will not be
--- unlocked. Which in turn will lead to deadlocks and timeouts. 
+-- unlocked. Which in turn will lead to deadlocks and timeouts.
+-- The callback can return any number of values, but only the first will be
+-- stored in the cache. 2nd and more results will only be returned when the
+-- first is `nil`.
 -- @param key the key under which to retrieve the data from the cache
 -- @param ttl time-to-live for the entry (in seconds)
 -- @param cb callback function. If no data is found under `key`, then the callback
--- is called with the additional parameters. The result from the callback is
--- then stored in the cache, and returned.
+-- is called with the additional parameters.
 -- @param ... the additional parameters passed to `cb`
--- @return the (newly) cached value
+-- @return the (newly) cached value, or if `nil` it returns all results from the
+-- callback
 function _M.get_or_set(key, ttl, cb, ...)
 
   -- Try to get the value from the cache
@@ -147,42 +157,53 @@ function _M.get_or_set(key, ttl, cb, ...)
     timeout = 5
   })
   if not lock then
-    ngx_log(ngx.ERR, "could not create lock: ", err)
+    log(ngx.ERR, "could not create lock: ", err)
     return
   end
 
   -- The value is missing, acquire a lock
   local elapsed, err = lock:lock(key)
   if not elapsed then
-    ngx_log(ngx.ERR, "failed to acquire cache lock: ", err)
+    log(ngx.ERR, "failed to acquire cache lock: ", err)
     return
   end
 
   -- Lock acquired. Since in the meantime another worker may have
-  -- populated the value we have to check again
+  -- populated the value we have to check again.
+  -- Errors will be collected, but only dealt with AFTER unlocking
   value = _M.get(key)
-  if value == nil then
-    -- Get from closure
-    value, err = cb(...)
-    if err then
-      return nil, err
+  if value ~= nil then
+    -- shm cache success
+    local ok, lerr = lock:unlock()
+    if not ok and lerr then
+      log(ngx.ERR, "failed to unlock: ", lerr)
     end
 
-    if value ~= nil then
-      local ok, err = _M.set(key, value, ttl)
-      if not ok then
-        ngx_log(ngx.ERR, err)
-        return
-      end
+    return value
+  end
+
+  -- cache failed, we need to invoke the callback
+  value = pack(cb(...))
+
+  if value[1] ~= nil then
+    local ok, err = _M.set(key, value[1], ttl)
+    if not ok then
+      log(ngx.ERR, err)
     end
   end
 
-  local ok, err = lock:unlock()
-  if not ok and err then
-    ngx_log(ngx.ERR, "failed to unlock: ", err)
+  local ok, lerr = lock:unlock()
+  if not ok and lerr then
+    log(ngx.ERR, "failed to unlock: ", lerr)
+  end
+
+  if value[1] ~= nil then
+    -- only return first value, as on each next lookup the cache will
+    -- also only return a single value
+    return value[1]
   end
 
-  return value
+  return unpack(value)
 end
 
 -- Utility Functions
diff --git a/kong/tools/utils.lua b/kong/tools/utils.lua
index 7b4c9807dbb..f889989cdce 100644
--- a/kong/tools/utils.lua
+++ b/kong/tools/utils.lua
@@ -57,6 +57,13 @@ _M.split = split
 -- @function strip
 _M.strip = strip
 
+--- packs a set of arguments in a table.
+-- Explicitly sets field `n` to the number of arguments, so it is `nil` safe
+_M.pack = function(...) return {n = select("#", ...), ...} end
+
+--- unpacks a table to a list of arguments.
+-- Explicitly honors the `n` field if given in the table, so it is `nil` safe
+_M.unpack = function(t, i, j) return unpack(t, i or 1, j or t.n or #t) end
 
 --- Retrieves the hostname of the local machine
 -- @return string  The hostname
diff --git a/spec/01-unit/04-utils_spec.lua b/spec/01-unit/04-utils_spec.lua
index 298dd11558e..279caba4883 100644
--- a/spec/01-unit/04-utils_spec.lua
+++ b/spec/01-unit/04-utils_spec.lua
@@ -486,4 +486,32 @@ describe("Utils", function()
       end
     end
   end)
+  it("pack() stores results, including nils, properly", function()
+    assert.same({ n = 0 }, utils.pack())
+    assert.same({ n = 1 }, utils.pack(nil))
+    assert.same({ n = 3, "1", "2", "3" }, utils.pack("1", "2", "3"))
+    assert.same({ n = 3, [1] = "1", [3] = "3" }, utils.pack("1", nil, "3"))
+  end)
+  it("unpack() unwraps results, including nils, properly", function()
+    local a,b,c
+    a,b,c = utils.unpack({})
+    assert.is_nil(a)
+    assert.is_nil(b)
+    assert.is_nil(c)
+
+    a,b,c = unpack({ n = 1 })
+    assert.is_nil(a)
+    assert.is_nil(b)
+    assert.is_nil(c)
+
+    a,b,c = utils.unpack({ n = 3, "1", "2", "3" })
+    assert.equal("1", a)
+    assert.equal("2", b)
+    assert.equal("3", c)
+
+    a,b,c = utils.unpack({ n = 3, [1] = "1", [3] = "3" })
+    assert.equal("1", a)
+    assert.is_nil(b)
+    assert.equal("3", c)
+  end)
 end)
diff --git a/spec/01-unit/11-database_cache_spec.lua b/spec/01-unit/11-database_cache_spec.lua
index 978c86a9ebe..fe63feb190b 100644
--- a/spec/01-unit/11-database_cache_spec.lua
+++ b/spec/01-unit/11-database_cache_spec.lua
@@ -107,5 +107,32 @@ describe("Database cache", function()
       -- verify
       assert.is.Nil(cache.get(key))
     end)
+
+    it("get_or_set only returns a single value on success", function()
+      local cb = function() return 1,2,3,4 end
+      local a,b,c,d = cache.get_or_set("just some key", nil, cb)
+      assert.equal(1, a)
+      assert.is_nil(b)
+      assert.is_nil(c)
+      assert.is_nil(d)
+
+      -- try again, while retrieving the cached value
+      local cb = function() return "result",2,3,4 end
+      local a,b,c,d = cache.get_or_set("just some key", nil, cb)
+      assert.equal(1, a)  -- still 1, cached value
+      assert.is_nil(b)
+      assert.is_nil(c)
+      assert.is_nil(d)
+    end)
+
+    it("get_or_set returns all values on failure", function()
+      local cb = function() return nil,2,3,4 end
+      local a,b,c,d = cache.get_or_set("just some other key", nil, cb)
+      assert.is_nil(a)
+      assert.equal(2, b)
+      assert.equal(3, c)
+      assert.equal(4, d)
+    end)
+
   end)
 end)