[tarantool-patches] [PATCH 2/4] swim: fix a dangerous yield in ffi.gc

[Vladislav Shpilevoy]

FFI can't survive yields. A yield in ffi.C.func() leads to a
crash; yield in ffi.gc is not documented as allowed. Yield in any
GC function leads to garbage collector stuck until the yield is
finished.

This patch makes SWIM GC callback non-yielding. Now yielding
swim_delete() is called in a separate fiber created in GC
callback, but started at the end of event loop only.

Follow up #3234
---
 src/lua/swim.lua        | 18 +++++++++++++++++-
 test/swim/swim.result   | 30 ++++++++++++++++++++++++++++++
 test/swim/swim.test.lua | 12 ++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/lua/swim.lua b/src/lua/swim.lua
index bae6c0da6..686c376cc 100644
--- a/src/lua/swim.lua
+++ b/src/lua/swim.lua
@@ -3,6 +3,7 @@ local uuid = require('uuid')
 local buffer = require('buffer')
 local msgpack = require('msgpack')
 local crypto = require('crypto')
+local fiber = require('fiber')
 local internal = require('swim')
 
 ffi.cdef[[
@@ -948,6 +949,21 @@ swim_cfg_not_configured_mt.__call = swim_cfg_first_call
 -- removed members erasure - GC drops them automatically.
 local cache_table_mt = { __mode = 'v' }
 
+--
+-- SWIM garbage collection function. It can't delete the SWIM
+-- instance immediately, because it is invoked by Lua GC. Firstly,
+-- it is not safe to yield in FFI - Jit can't survive a yield.
+-- Secondly, it is not safe to yield in any GC function, because
+-- it stops garbage collection. Instead, here a new fiber is
+-- created without yields, which works at the end of the event
+-- loop, and deletes the instance asynchronously.
+--
+local function swim_gc(ptr)
+    fiber.new(function()
+        internal.swim_delete(ptr)
+    end)
+end
+
 --
 -- Create a new SWIM instance, and configure if @a cfg is
 -- provided.
@@ -969,7 +985,7 @@ local function swim_new(cfg)
     if ptr == nil then
         return nil, box.error.last()
     end
-    ffi.gc(ptr, internal.swim_delete)
+    ffi.gc(ptr, swim_gc)
     local s = setmetatable({
         ptr = ptr,
         cfg = setmetatable({index = {}}, swim_cfg_not_configured_mt),
diff --git a/test/swim/swim.result b/test/swim/swim.result
index 6004971d0..318b4bed9 100644
--- a/test/swim/swim.result
+++ b/test/swim/swim.result
@@ -1595,6 +1595,36 @@ s
 ---
 - []
 ...
+--
+-- Check that SWIM GC doesn't block nor crash garbage collector.
+--
+s = swim.new()
+---
+...
+allow_gc = false
+---
+...
+_ = s:on_member_event(function() while not allow_gc do pcall(fiber.sleep, 0.01) end end)
+---
+...
+s:cfg({uri = 0, uuid = uuid(1)})
+---
+- true
+...
+s = setmetatable({s}, {__mode = 'v'})
+---
+...
+collectgarbage('collect')
+---
+- 0
+...
+s
+---
+- []
+...
+allow_gc = true
+---
+...
 test_run:cmd("clear filter")
 ---
 - true
diff --git a/test/swim/swim.test.lua b/test/swim/swim.test.lua
index f1139087c..16b77b602 100644
--- a/test/swim/swim.test.lua
+++ b/test/swim/swim.test.lua
@@ -545,4 +545,16 @@ s = setmetatable({s}, {__mode = 'v'})
 collectgarbage('collect')
 s
 
+--
+-- Check that SWIM GC doesn't block nor crash garbage collector.
+--
+s = swim.new()
+allow_gc = false
+_ = s:on_member_event(function() while not allow_gc do pcall(fiber.sleep, 0.01) end end)
+s:cfg({uri = 0, uuid = uuid(1)})
+s = setmetatable({s}, {__mode = 'v'})
+collectgarbage('collect')
+s
+allow_gc = true
+
 test_run:cmd("clear filter")
-- 
2.20.1 (Apple Git-117)