gc: prevent potential fiber yield

Yielding in gc is prohibited since Tarantool 2.6.0-138-gd3f1dd720,
2.5.1-105-gc690b3337, 2.4.2-89-g83037df15, 1.10.7-47-g8099cb053.
If fiber yield happens in object gc, Tarantool process exits with
code 1.

Gc hook of a connection from a connection pool calls
`pool.queue:put(...)`. Fiber `channel:put()` may yield if the channel
is full. `channel:put()`/`channel:get()` in mysql driver connection pool
is balanced:
* the channel capacity is equal to the connection count;
* pool create fills the channel with multiple `channel:put()`s
  all the way through;
* `pool:get()` causes single `channel:get()` and creates a connection
   object with gc hook;
* `pool:put()` causes single `channel:put()` and removes gc hook;
* gc hook causes single `channel:put()`.

There are no other cases of `channel:put()`. `pool:put()` and gc hook
cannot be executed both in the same time: either a connection is used
inside `pool:put()` and won't be garbage collected or a connection is no
longer used anywhere else (including `pool:put()`) and it will be
garbage collected. So now there are no valid cases when gc hook may
yield unless someone messes up with pool queue. Messing up with pool
queue anyway breaks a connection pool internal logic, so we make the
pool unavailable in that case.

Delayed gc should not break existing logic. If `pool:get()` is called,
we expect to get a connection after gc and it's not yet returned,
`channel:get()` will yield and trigger all planned returns.

Closes #67

Author: DifferentialOrange

mysql/init.lua

@@ -3,6 +3,7 @@
 local fiber = require('fiber')
 local driver = require('mysql.driver')
 local ffi = require('ffi')
+local log = require('log')
 
 local pool_mt
 local conn_mt
@@ -25,6 +26,22 @@ local function conn_create(mysql_conn)
     return conn
 end
 
+-- There is no reason to make it configurable: either everyting is alright and
+-- channel put is immediate or pool is doomed.
+local CONN_GC_HOOK_TIMEOUT = 0
+
+local function conn_gc_hook(pool)
+    local success = pool.queue:put(POOL_EMPTY_SLOT, CONN_GC_HOOK_TIMEOUT)
+    if not success then
+        log.error('pool %s internal queue unexpected state: there are no ' ..
+                  'empty slots. It is likely that someone had messed with ' ..
+                  'pool.queue manually. Closing the pool...')
+        log.error(debug.traceback)
+
+        pool:close()
+    end
+end
+
 -- get connection from pool
 local function conn_get(pool, timeout)
     local mysql_conn = pool.queue:get(timeout)
@@ -48,7 +65,12 @@ local function conn_get(pool, timeout)
     conn.__gc_hook = ffi.gc(ffi.new('void *'),
             function(self)
                 mysql_conn:close()
-                pool.queue:put(POOL_EMPTY_SLOT)
+                -- Fiber yields are prohibited in gc since Tarantool
+                -- * 2.6.0-138-gd3f1dd720
+                -- * 2.5.1-105-gc690b3337
+                -- * 2.4.2-89-g83037df15
+                -- * 1.10.7-47-g8099cb053
+                fiber.new(conn_gc_hook, pool)
             end)
     -- If the connection belongs to a connection pool, it must be returned to
     -- the pool when calling "close" without actually closing the connection.

test/mysql.test.lua

@@ -91,6 +91,9 @@ local function test_gc(test, pool)
         -- Collect lost connections.
         collectgarbage('collect')
 
+        -- Run a fiber scheduler cycle to finish gc process.
+        fiber.yield()
+
         -- Verify that a pool is aware of collected connections.
         test:is(pool.queue:count(), pool.size, 'all connections are put back')
     end)
@@ -111,6 +114,9 @@ local function test_gc(test, pool)
         -- Collect the lost connection.
         collectgarbage('collect')
 
+        -- Run a fiber scheduler cycle to finish gc process.
+        fiber.yield()
+
         -- Verify that a pool is aware of collected connections.
         test:is(pool.queue:count(), pool.size, 'all connections are put back')
     end)
@@ -129,6 +135,9 @@ local function test_gc(test, pool)
         -- Collect the lost connection.
         collectgarbage('collect')
 
+        -- Run a fiber scheduler cycle to finish gc process.
+        fiber.yield()
+
         -- Verify that a pool is aware of collected connections.
         test:is(pool.queue:count(), pool.size, 'all connections are put back')
     end)
@@ -646,8 +655,63 @@ local function test_put_to_wrong_pool(test)
               format(p1, p2))
 end
 
+-- gh-67: Check that garbage collection of a connection from a connection pool
+-- not yields. Yielding in gc is prohibited since Tarantool
+-- 2.6.0-138-gd3f1dd720, 2.5.1-105-gc690b3337, 2.4.2-89-g83037df15,
+-- 1.10.7-47-g8099cb053. If fiber yield happens in object gc, Tarantool process
+-- exits with code 1.
+--
+-- Gc hook of a connection from a connection pool calls `pool.queue:put(...)`.
+-- Fiber `channel:put()` may yield if the channel is full.
+-- `channel:put()`/`channel:get()` in mysql driver connection pool is balanced:
+-- * the channel capacity is equal to the connection count;
+-- * pool create fills the channel with multiple `channel:put()`s
+--   all the way through;
+-- * `pool:get()` causes single `channel:get()` and creates a connection object
+--    with gc hook;
+-- * `pool:put()` causes single `channel:put()` and removes gc hook;
+-- * gc hook causes single `channel:put()`.
+--
+-- There are no other cases of `channel:put()`. `pool:put()` and gc hook cannot
+-- be executed both in the same time: either a connection is used inside
+-- `pool:put()` and won't be garbage collected or a connection is no longer used
+-- anywhere else (including `pool:put()`) and it will be garbage collected.
+-- So now there are no valid cases when gc hook may yield unless someone messes
+-- up with pool queue. Messing up with pool queue anyway breaks a connection
+-- pool internal logic, but at least it won't be causing a process exit.
+local function test_conn_from_pool_gc_yield(test)
+    test:plan(4)
+
+    local p = mysql.pool_create({
+        host = host,
+        port = port,
+        user = user,
+        password = password,
+        db = db,
+        size = 1
+    })
+
+    local c = p:get()
+    local res = c:ping()
+    test:ok(res, 'Connection is ok')
+    p.queue:put('killer msg')
+
+    c = nil -- luacheck: no unused
+    collectgarbage('collect')
+    collectgarbage('collect')
+
+    -- Run a fiber scheduler cycle to finish gc process.
+    fiber.yield()
+
+    test:ok(true, 'Tarantool is alive')
+
+    local ok, err = pcall(p.get, p)
+    test:is(ok, false, 'Pool is not usable')
+    test:like(tostring(err), 'Pool is not usable')
+end
+
 local test = tap.test('mysql connector')
-test:plan(11)
+test:plan(12)
 
 test:test('connection old api', test_old_api, conn)
 local pool_conn = p:get()
@@ -662,6 +726,7 @@ test:test('test_underlying_conn_closed_during_gc',
 test:test('ffi null printing', test_ffi_null_printing, p)
 test:test('test_block_fiber_inf', test_block_fiber_inf, p)
 test:test('test_put_to_wrong_pool', test_put_to_wrong_pool)
+test:test('test_conn_from_pool_gc_yield', test_conn_from_pool_gc_yield)
 p:close()
 
 os.exit(test:check() and 0 or 1)