[balancer] fix memory leak in round-robin balancer

before:

/016-balancer.t .. TEST 1: round robin does not leak memory
WARNING: method GET not supported for ab when taking a request body
LeakTest: [5768 5964 5964 6192 6224 6224 6500 6500 6500 6760 6788 6788 6928 7088 7088 7088 7532 8300 8300 8300 8300 8300 8856 9672 10084 10084 10084 10084 10084 10584 10584 10584 10584 10584 10776 11112 11112 11112 11112 11112 11460 11668 11668 11668 11668 11668 11872 12256 12256 12256 12256 12256 12256 12732 12872 12872 13128 14664 14664 14664 14664 14664 14664 14664 14664 15168 15968 16740 17500 18088 18164 18164 18164 18164 18164 18164 18164 18164 18164 18164 18460 19080 19144 19144 19144 19144 19144 19144 19144 19144 19144 19144 19144 19636 20164 20164 20164 20164 20164 20164]
LeakTest: k=158.8
t/016-balancer.t .. ok
All tests successful.
Files=1, Tests=4,  6 wallclock secs ( 0.03 usr  0.00 sys +  1.21 cusr  3.91 csys =  5.15 CPU)
Result: PASS

after:

t/016-balancer.t .. TEST 1: round robin does not leak memory
WARNING: method GET not supported for ab when taking a request body
LeakTest: [2248 2248 2248 2248 2248 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252 2252]
LeakTest: k=0.0
t/016-balancer.t .. ok
All tests successful.
Files=1, Tests=4,  6 wallclock secs ( 0.02 usr  0.01 sys +  1.15 cusr  3.55 csys =  4.73 CPU)
Result: PASS

CHANGELOG.md

@@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Memory leak in every request [PR #339](https://github.com/3scale/apicast/pull/339)
 - Remove unnecessary code and comments [PR #344](https://github.com/3scale/apicast/pull/344)
 - JWT expiry not taken into account in authorization response cache [PR #283](https://github.com/3scale/apicast/pull/283) / [Issue #309](https://github.com/3scale/apicast/issues/309) / Fixed by [PR #341](https://github.com/3scale/apicast/pull/341)
+- Memory leak in round robin balancer [PR #345](https://github.com/3scale/apicast/pull/345)
 
 ## [3.0.0-beta3] - 2017-03-20
 

apicast/src/resty/balancer/round_robin.lua

@@ -3,25 +3,26 @@ local min = math.min
 local random = math.random
 
 local balancer = require 'resty.balancer'
-local semaphore = require 'ngx.semaphore'
 local balancer_random = require 'resty.balancer.random'
+local lrucache = require 'resty.lrucache'
 
 local _M = {
-  _VERSION = '0.1'
+  _VERSION = '0.1',
+  cache_size = 1000
 }
 
-local call = semaphore.new(1)
-
-local cursor = {}
+local cursor
 
 function _M.new()
   return balancer.new(_M.call)
 end
 
 function _M.reset()
-  cursor = {}
+  cursor = lrucache.new(_M.cache_size)
 end
 
+_M.reset()
+
 function _M.call(peers)
   if #peers == 0 then
     return nil, 'empty peers'
@@ -33,15 +34,14 @@ function _M.call(peers)
     return balancer_random.call(peers)
   end
 
-  local ok, _ = call:wait(0)
-
+  -- This looks like there might be a race condition but I think it is not.
+  -- The VM will not schedule another thread until there is IO and in this case there is no IO.
+  -- So this block stays the only one being executed and no one can access the same cursor value.
   local n = #peers
-  local i = min(max(cursor[hash] or peers.cur or random(1, n), 0), n)
+  local i = min(max(cursor:get(hash) or peers.cur or random(1, n), 0), n)
   local peer = peers[i]
 
-  cursor[hash] = (i % n) + 1
-
-  if ok then call:post(1) end
+  cursor:set(hash, (i % n) + 1)
 
   return peer, i
 end

t/016-balancer.t

@@ -0,0 +1,46 @@
+use Test::Nginx::Socket 'no_plan';
+use Cwd qw(cwd);
+
+my $pwd = cwd();
+my $apicast = $ENV{TEST_NGINX_APICAST_PATH} || "$pwd/apicast";
+
+$ENV{TEST_NGINX_LUA_PATH} = "$apicast/src/?.lua;;";
+$ENV{TEST_NGINX_HTTP_CONFIG} = "$apicast/http.d/*.conf";
+$ENV{TEST_NGINX_APICAST_PATH} = $apicast;
+$ENV{RESOLVER} = '127.0.1.1:5353';
+
+
+env_to_nginx(
+    'RESOLVER'
+);
+master_on();
+log_level('debug');
+repeat_each(1);
+no_root_location();
+run_tests();
+
+__DATA__
+
+=== TEST 1: round robin does not leak memory
+Balancing different hosts does not leak memory.
+--- http_config
+  lua_package_path "$TEST_NGINX_LUA_PATH";
+  init_by_lua_block {
+    require('resty.balancer.round_robin').cache_size = 1
+  }
+--- config
+  location = /t {
+    content_by_lua_block {
+      local round_robin = require('resty.balancer.round_robin')
+      local balancer = round_robin.new()
+
+      local peers = { hash = ngx.var.request_id, cur = 1,  1, 2 }
+      local peer = round_robin.call(peers)
+
+      ngx.print(peer)
+    }
+  }
+--- pipelined_requests eval
+[ "GET /t", "GET /t" ]
+--- response_body eval
+[ "1", "1" ]