From 1645e1b7b8278f98fa6e9cf3cb9114880627dd51 Mon Sep 17 00:00:00 2001
From: better0fdead <better0fdead@tarantool.org>
Date: Wed, 4 Oct 2023 12:18:56 +0300
Subject: [PATCH] readview: fix 'garbagecollector'

Added workaround for 'garbagecollector' of readview because
for tarantool lua (and lua 5.1) '__gc' metamethod only works for cdata types.

See https://github.com/tarantool/tarantool/issues/5770
---
 CHANGELOG.md                              |  5 ++
 crud/readview.lua                         | 10 ++++
 test/integration/select_readview_test.lua | 67 ++++++++++++++++++++---
 3 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0daa0c83..0f47c19b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+* `crud.readview` resource cleanup on garbage collect (#379).
+
 ## [1.3.0] - 27-09-23
 
 ### Added
diff --git a/crud/readview.lua b/crud/readview.lua
index 777000ed..5a9d7251 100644
--- a/crud/readview.lua
+++ b/crud/readview.lua
@@ -285,7 +285,17 @@ end
 
 function Readview_obj.create(vshard_router, opts)
     local readview = {}
+
+    -- For tarantool lua (and lua 5.1) __gc metamethod only works for cdata types.
+    -- So in order to create a proper GC hook, we need to create cdata with
+    -- __gc call.
+    -- __gc call for this cdata will be a __gc call for our readview.
+    -- https://github.com/tarantool/tarantool/issues/5770
+    local proxy = newproxy(true)
+    getmetatable(proxy).__gc = function(_) Readview_obj.__gc(readview) end
+    readview[proxy] = true
     setmetatable(readview, Readview_obj)
+
     readview._name = opts.name
     local results, err, err_uuid = vshard_router:map_callrw('_crud.readview_open_on_storage',
         {readview._name}, {timeout = opts.timeout})
diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua
index 6134faff..bfa8c04a 100644
--- a/test/integration/select_readview_test.lua
+++ b/test/integration/select_readview_test.lua
@@ -1,4 +1,5 @@
 local fio = require('fio')
+local fiber = require('fiber')
 
 local t = require('luatest')
 
@@ -176,7 +177,7 @@ pgroup.test_gc_on_storage = function(g)
     })
 end
 
-pgroup.test_close_gc_on_router = function(g)
+pgroup.test_gc_rv_not_referenced_on_router = function(g)
     local _, err = g.cluster.main_server.net_box:eval([[
         local crud = require('crud')
         local foo, err = crud.readview({name = 'foo'})
@@ -187,17 +188,65 @@ pgroup.test_close_gc_on_router = function(g)
         collectgarbage("collect")
         collectgarbage("collect")
     ]])
+    fiber.sleep(1)
     t.assert_equals(err, nil)
     local res = {}
-    helpers.call_on_storages(g.cluster, function(server, replicaset, res)
+    helpers.call_on_storages(g.cluster, function(server)
         local instance_res = server.net_box:eval([[
         return box.read_view.list()]])
-        res[replicaset.alias] = instance_res
-     end, res)
-    t.assert_equals(res, {["s-1"] = {}, ["s-2"] = {}})
+        table.insert(res, instance_res)
+    end, res)
+    t.assert_equals(res, {{}, {}, {}, {}})
 
 end
 
+pgroup.test_gc_rv_referenced_on_router = function(g)
+    helpers.insert_objects(g, 'customers', {
+        {
+            id = 1, name = "Elizabeth", last_name = "Jackson",
+            age = 12, city = "New York",
+        }, {
+            id = 2, name = "Mary", last_name = "Brown",
+            age = 46, city = "Los Angeles",
+        }, {
+            id = 3, name = "David", last_name = "Smith",
+            age = 33, city = "Los Angeles",
+        }, {
+            id = 4, name = "William", last_name = "White",
+            age = 81, city = "Chicago",
+        },
+    })
+
+    local _, err = g.cluster.main_server.net_box:eval([[
+        local crud = require('crud')
+        local foo, err = crud.readview({name = 'foo'})
+        if err ~= nil then
+            return nil, err
+        end
+        collectgarbage("collect")
+        collectgarbage("collect")
+        rawset(_G, 'foo', foo)
+    ]])
+    fiber.sleep(1)
+    t.assert_equals(err, nil)
+    local obj, err = g.cluster.main_server.net_box:eval([[
+        local crud = require('crud')
+        local foo = rawget(_G, 'foo')
+        local result, err = foo:select('customers', nil, {fullscan = true})
+
+        foo:close()
+        return result, err
+    ]])
+
+    t.assert_equals(err, nil)
+    t.assert_equals(obj.rows, {
+        {1, 477, "Elizabeth", "Jackson", 12, "New York"},
+        {2, 401, "Mary", "Brown", 46, "Los Angeles"},
+        {3, 2804, "David", "Smith", 33, "Los Angeles"},
+        {4, 1161, "William", "White", 81, "Chicago"},
+    })
+end
+
 pgroup.test_close = function(g)
     local _, err = g.cluster.main_server.net_box:eval([[
         local crud = require('crud')
@@ -209,12 +258,12 @@ pgroup.test_close = function(g)
     ]])
     t.assert_equals(err, nil)
     local res = {}
-    helpers.call_on_storages(g.cluster, function(server, replicaset, res)
+    helpers.call_on_storages(g.cluster, function(server)
         local instance_res = server.net_box:eval([[
         return box.read_view.list()]])
-        res[replicaset.alias] = instance_res
-     end, res)
-    t.assert_equals(res, {["s-1"] = {}, ["s-2"] = {}})
+        table.insert(res, instance_res)
+    end, res)
+    t.assert_equals(res, {{}, {}, {}, {}})
 
 end