Fix strong reference race in GC (#65)

* Fix strong reference race in GC
* Add regress test
* Fix type stored in StoredObject
* Fix thread input arguments lifetime

src/cpp/channel.cpp

@@ -36,6 +36,7 @@ bool Channel::push(const sol::variadic_args& args) {
         auto obj = createStoredObject(arg.get<sol::object>());
         if (obj->gcHandle())
             refs_->insert(obj->gcHandle());
+        obj->releaseStrongReference();
         array.emplace_back(obj);
     }
     if (data_->channel_.empty())

src/cpp/garbage-collector.h

@@ -22,6 +22,7 @@ public:
             : refs_(new std::set<GCObjectHandle>) {}
     GCObject(const GCObject& init) = default;
     GCObject(GCObject&& init) = default;
+    GCObject& operator=(const GCObject& init) = default;
     virtual ~GCObject() = default;
 
     GCObjectHandle handle() const noexcept { return reinterpret_cast<GCObjectHandle>(refs_.get()); }

src/cpp/shared-table.cpp

@@ -54,6 +54,8 @@ void SharedTable::set(StoredObject&& key, StoredObject&& value) {
         refs_->insert(key->gcHandle());
     if (value->gcHandle())
         refs_->insert(value->gcHandle());
+    key->releaseStrongReference();
+    value->releaseStrongReference();
 
     data_->entries[std::move(key)] = std::move(value);
 }

src/cpp/shared-table.h

@@ -22,6 +22,7 @@ public:
     SharedTable();
     SharedTable(SharedTable&&) = default;
     SharedTable(const SharedTable& init);
+    SharedTable& operator=(const SharedTable&) = default;
     virtual ~SharedTable() = default;
 
     static void getUserType(sol::state_view& lua);

src/cpp/stored-object.cpp

@@ -74,12 +74,15 @@ public:
     template <typename SolType>
     GCObjectHolder(const SolType& luaObject) {
         assert(luaObject.template is<T>());
-        handle_ = luaObject.template as<T>().handle();
+        strongRef_ = luaObject.template as<T>();
+        handle_ = strongRef_->handle();
         assert(GC::instance().has(handle_));
     }
 
     GCObjectHolder(GCObjectHandle handle)
-            : handle_(handle) {}
+            : handle_(handle) {
+        strongRef_ = GC::instance().get<T>(handle_);
+    }
 
     bool rawCompare(const BaseHolder* other) const final {
         return handle_ < static_cast<const GCObjectHolder<T>*>(other)->handle_;
@@ -91,8 +94,13 @@ public:
 
     GCObjectHandle gcHandle() const override { return handle_; }
 
+    void releaseStrongReference() override {
+        strongRef_ = sol::nullopt;
+    }
+
 private:
     GCObjectHandle handle_;
+    sol::optional<T> strongRef_;
 };
 
 // This class is used as a storage for visited sol::tables

src/cpp/stored-object.h

@@ -22,6 +22,7 @@ public:
     virtual const std::type_info& type() { return typeid(*this); }
     virtual sol::object unpack(sol::this_state state) const = 0;
     virtual GCObjectHandle gcHandle() const { return GCNull; }
+    virtual void releaseStrongReference() { }
 
 private:
     BaseHolder(const BaseHolder&) = delete;

src/cpp/threading.cpp

@@ -171,10 +171,11 @@ void runThread(std::shared_ptr<ThreadHandle> handle,
 
     try {
         {
-            ScopeGuard reportComplete([=](){
+            ScopeGuard reportComplete([handle, &arguments](){
                 DEBUG << "Finished " << std::endl;
                 // Let's destroy accociated state
                 // to release all resources as soon as possible
+                arguments.clear();
                 handle->destroyLua();
             });
             sol::function userFuncObj = loadString(handle->lua(), strFunction);

tests/lua/gc-stress.lua

@@ -0,0 +1,40 @@
+require "bootstrap-tests"
+
+test.gc_stress.tear_down = default_tear_down
+
+-- Regress test for simultaneous object creation and removing
+-- may cause SIGFAULT, so it's marked as "stress"
+test.gc_stress.create_and_collect_in_parallel = function ()
+    function worker()
+        effil = require "effil"
+        local nested_table = {
+                {}, --[[1 level]]
+                {{}}, --[[2 levels]]
+                {{{}}}, --[[3 levels]]
+                {{{{}}}} --[[4 levels]]
+        }
+        for i = 1, 100 do
+            for t = 1, 10 do
+                local tbl = effil.table(nested_table)
+                for l = 1, 10 do
+                    tbl[l] = nested_table
+                end
+            end
+            collectgarbage()
+            effil.gc.collect()
+        end
+    end
+
+    local thread_num = 10
+    local threads = {}
+
+    for i = 1, thread_num do
+        threads[i] = effil.thread(worker)(i)
+    end
+
+    for i = 1, thread_num do
+        test.equal(threads[i]:wait(), "completed")
+    end
+
+    test.equal(effil.gc.count(), 1)
+end

tests/lua/tests.lua

@@ -12,6 +12,7 @@ require "metatable"
 if os.getenv("STRESS") then
     require "channel-stress"
     require "thread-stress"
+    require "gc-stress"
 end
 
 test.summary()