update connection management for connection pool

If the connection belongs to a connection pool, it must be
returned to the pool when calling "close" without actually
closing the connection. All connections will be closed when
poll:close() will be called. The same behavior is used in
JDBC connection pool.
With this change, an attempt to merge an unusable connection
is no longer valid. But before it was valid, so now it's just
a NOP.

Fixed #33

Author: LeonidVas

mysql/init.lua

@@ -52,6 +52,17 @@ local function conn_get(pool, timeout)
                 mysql_conn:close()
                 pool.queue:put(POOL_EMPTY_SLOT)
             end)
+    -- If the connection belongs to a connection pool, it must be returned to
+    -- the pool when calling "close" without actually closing the connection.
+    -- In the case of a double "close", the behavior is the same as with a
+    -- simple connection.
+    conn.close = function(self)
+        if not self.usable then
+            error('Connection is not usable')
+        end
+        pool:put(self)
+        return true
+    end
     return conn
 end
 
@@ -209,8 +220,6 @@ end
 local function pool_put(self, conn)
     if conn.usable then
         self.queue:put(conn_put(conn))
-    else
-        self.queue:put(POOL_EMPTY_SLOT)
     end
 end
 

test/mysql.test.lua

@@ -171,7 +171,7 @@ local function test_mysql_int64(t, p)
 end
 
 local function test_connection_pool(test, pool)
-    test:plan(6)
+    test:plan(11)
 
     -- {{{ Case group: all connections are consumed initially.
 
@@ -302,13 +302,13 @@ local function test_connection_pool(test, pool)
 
         -- Put the connection back and verify that the pool is full.
         pool:put(conn)
-        test:ok(pool.queue:is_full(), 'a broken connection was given back')
+        test:ok(pool.queue:is_full(), 'a connection was given back')
     end)
 
     -- Case: the same, but loss and collect a connection after
     -- put.
     test:test('get, put and loss a connection', function(test)
-        test:plan(2)
+        test:plan(1)
 
         assert(pool.size >= 1, 'test case precondition fails')
 
@@ -322,13 +322,6 @@ local function test_connection_pool(test, pool)
 
         -- Verify that the pool is full.
         test:ok(pool.queue:is_full(), 'a broken connection was given back')
-
-        -- Verify the pool will not be populated by a connection's
-        -- GC callback. Otherwise :put() will hang.
-        local item = pool.queue:get()
-        pool.queue:put(item)
-        test:ok(true, 'GC callback does not put back a connection that was ' ..
-            'put manually')
     end)
 
     -- Case: get a connection, broke it and put back.
@@ -350,7 +343,7 @@ local function test_connection_pool(test, pool)
     -- Case: the same, but loss and collect a connection after
     -- put.
     test:test('get, broke, put and loss a connection', function(test)
-        test:plan(3)
+        test:plan(2)
 
         assert(pool.size >= 1, 'test case precondition fails')
 
@@ -366,24 +359,10 @@ local function test_connection_pool(test, pool)
 
         -- Verify that the pool is full
         test:ok(pool.queue:is_full(), 'a broken connection was given back')
-
-        -- Verify the pool will not be populated by a connection's
-        -- GC callback. Otherwise :put() will hang.
-        local item = pool.queue:get()
-        pool.queue:put(item)
-        test:ok(true, 'GC callback does not put back a connection that was ' ..
-            'put manually')
     end)
 
-    --[[
-
-    -- It is unclear for now whether putting of closed connection
-    -- should be allowed. The second case, where GC collects lost
-    -- connection after :put(), does not work at the moment. See
-    -- gh-33.
-
-    -- Case: get a connection, close it and put back.
-    test:test('get, close and put a connection', function(test)
+    -- Case: get a connection and close it.
+    test:test('get and close a connection', function(test)
         test:plan(1)
 
         assert(pool.size >= 1, 'test case precondition fails')
@@ -392,39 +371,73 @@ local function test_connection_pool(test, pool)
         local conn = pool:get()
         conn:close()
 
-        -- Put a connection back and verify that the pool is full.
-        pool:put(conn)
-        test:ok(pool.queue:is_full(), 'a broken connection was given back')
+        test:ok(pool.queue:is_full(), 'a connection was given back')
     end)
 
     -- Case: the same, but loss and collect a connection after
-    -- put.
-    test:test('get, close, put and loss a connection', function(test)
-        test:plan(2)
+    -- close.
+    test:test('get, close and loss a connection', function(test)
+        test:plan(1)
 
         assert(pool.size >= 1, 'test case precondition fails')
 
         -- Get a connection and close it.
         local conn = pool:get()
         conn:close()
 
-        -- Put the connection back, loss it and trigger GC.
-        pool:put(conn)
-        conn = nil
+        conn = nil -- luacheck: no unused
         collectgarbage('collect')
 
         -- Verify that the pool is full
         test:ok(pool.queue:is_full(), 'a broken connection was given back')
+    end)
+
+    -- Case: get a connection, close and put it back.
+    test:test('get, close and put a connection', function(test)
+        test:plan(1)
 
-        -- Verify the pool will not be populated by a connection's
-        -- GC callback. Otherwise :put() will hang.
-        local item = pool.queue:get()
-        pool.queue:put(item)
-        test:ok(true, 'GC callback does not put back a connection that was ' ..
-            'put manually')
+        assert(pool.size >= 1, 'test case precondition fails')
+
+        -- Get a connection.
+        local conn = pool:get()
+
+        conn:close()
+        -- Put must be a NOP in this case.
+        pool:put(conn)
+        test:ok(pool.queue:is_full(), 'a connection was given back')
+    end)
+
+     -- Case: close the same connection twice.
+    test:test('close a connection twice', function(test)
+        test:plan(2)
+
+        assert(pool.size >= 1, 'test case precondition fails')
+
+        local conn = pool:get()
+        conn:close()
+        test:ok(pool.queue:is_full(), 'a connection was given back')
+
+        local res = pcall(conn.close, conn)
+        test:ok(not res, 'an error is thrown on double "close"')
     end)
 
-    --]]
+    -- Case: put the same connection twice.
+    test:test('put a connection twice', function(test)
+        test:plan(2)
+
+        assert(pool.size >= 2, 'test case precondition fails')
+
+        local conn_1 = pool:get()
+        local conn_2 = pool:get()
+        pool:put(conn_1)
+        pool:put(conn_1)
+        test:ok(not pool.queue:is_full(),
+                'the same connection has not been "put" twice')
+
+        pool:put(conn_2)
+        test:ok(pool.queue:is_full(),
+                'all connections were returned to the pool')
+    end)
 
     assert(pool.queue:is_full(), 'case group postcondition fails')
 
@@ -437,7 +450,6 @@ test:plan(6)
 test:test('connection old api', test_old_api, conn)
 local pool_conn = p:get()
 test:test('connection old api via pool', test_old_api, pool_conn)
-p:put(pool_conn)
 test:test('garbage collection', test_gc, p)
 test:test('concurrent connections', test_conn_concurrent, p)
 test:test('int64', test_mysql_int64, p)