fix(least-conn) change cleanup order (#126)

remove address from binary heap upon 'disabling' instead of when
'deleting'.

README.md

@@ -51,6 +51,11 @@ Release process:
 4. commit and tag the release
 5. upload rock to LuaRocks
 
+### unreleased
+
+- Fix: potential synchronisation issue in the least-connections balancer.
+  [PR 126](https://github.com/Kong/lua-resty-dns-client/pull/126)
+
 ### 5.2.2 (11-Mar-2021)
 
 - Fix: do not iterate over all the search domains when resolving an unambiguous

src/resty/dns/balancer/least_connections.lua

@@ -69,6 +69,13 @@ function lcAddr:setState(available)
 end
 
 
+-- disabling the address, so delete from binaryHeap
+function lcAddr:disable()
+  self.host.balancer.binaryHeap:remove(self)
+  self.super.disable(self)
+end
+
+
 function lc:newAddress(addr)
   addr = self.super.newAddress(self, addr)
 
@@ -89,12 +96,6 @@ function lc:newAddress(addr)
 end
 
 
--- removing the address, so delete from binaryHeap
-function lc:onRemoveAddress(address)
-  self.binaryHeap:remove(address)
-end
-
-
 
 function lc:getPeer(cacheOnly, handle, hashValue)
   if handle then
@@ -113,7 +114,7 @@ function lc:getPeer(cacheOnly, handle, hashValue)
     handle.retryCount = 0
   end
 
-  local address, ip, port, host, reinsert
+  local address, ip, port, host
   while true do
     if not self.healthy then
       -- Balancer unhealthy, nothing we can do.
@@ -126,6 +127,7 @@ function lc:getPeer(cacheOnly, handle, hashValue)
 
     -- go and find the next `address` object according to the LB policy
     do
+      local reinsert
       repeat
         if address then
           -- this address we failed before, so temp store it and pop it from
@@ -151,7 +153,7 @@ function lc:getPeer(cacheOnly, handle, hashValue)
           local addr = reinsert[i]
           self.binaryHeap:insert((addr.connectionCount + 1) / addr.weight, addr)
         end
-        reinsert = nil
+        reinsert = nil -- luacheck: ignore
       end
     end