Fix segfault with luaruntime destructor

6 files changed, 52 insertions, 44 deletions

diff --git a/spec/KeySeqParserSpec.cpp b/spec/KeySeqParserSpec.cpp
index 306f5ab..7ce2a4a 100644
--- a/spec/KeySeqParserSpec.cpp
+++ b/spec/KeySeqParserSpec.cpp
@@ -8,7 +8,7 @@ class KeySeqParserSpec : public QObject {
   Q_OBJECT
 
 private slots:
-  void testThings() {
+  void test_parse() {
     context("when mix of upper/lower cases");
     it("parses keys ignoring the casing") {
       KeySeqParser parser;
diff --git a/spec/KeymapEvaluatorSpec.cpp b/spec/KeymapEvaluatorSpec.cpp
index 4c19dfd..8e8ba87 100644
--- a/spec/KeymapEvaluatorSpec.cpp
+++ b/spec/KeymapEvaluatorSpec.cpp
@@ -7,7 +7,7 @@ class KeymapEvaluatorSpec : public QObject {
   Q_OBJECT
 
 private slots:
-  void testEvaluateSingleKeyChord() {
+  void test_evaluate_single_key_chord() {
     context("when the key sequence is mapped");
     it("calls mapping") {
       int keymapWasCalled = false;
@@ -33,7 +33,7 @@ private slots:
     }
   }
 
-  void testEvaluateMultiKeySequence() {
+  void test_evaluate_multi_key_sequence() {
     context("when the full key sequence is mapped");
     it("calls mapping") {
       int keymapWasCalled = false;
diff --git a/spec/LuaRuntimeSpec.cpp b/spec/LuaRuntimeSpec.cpp
index 0debda1..3b4be70 100644
--- a/spec/LuaRuntimeSpec.cpp
+++ b/spec/LuaRuntimeSpec.cpp
@@ -1,6 +1,4 @@
 #include <QtCore>
-#include <chrono>
-#include <thread>
 #include <uv.h>
 
 #include "LuaRuntime.hpp"
@@ -17,9 +15,9 @@ private slots:
     uv_library_shutdown();
   }
 
-  void testEvaluate() {
+  void test_evaluate() {
     context("when given an expression returning a number");
-    xit("emits evaluationCompleted with the result") {
+    it("emits evaluationCompleted with the result") {
       auto lua = LuaRuntime::instance();
       QSignalSpy evaluationCompletedSpy(lua, &LuaRuntime::evaluationCompleted);
 
@@ -32,7 +30,7 @@ private slots:
     }
 
     context("when given an expression returning a string");
-    xit("emits evaluationCompleted with the result") {
+    it("emits evaluationCompleted with the result") {
       auto lua = LuaRuntime::instance();
       QSignalSpy evaluationCompletedSpy(lua, &LuaRuntime::evaluationCompleted);
 
@@ -45,7 +43,7 @@ private slots:
     }
 
     context("when given an expression returning a boolean");
-    xit("emits evaluationCompleted with the result") {
+    it("emits evaluationCompleted with the result") {
       auto lua = LuaRuntime::instance();
       QSignalSpy evaluationCompletedSpy(lua, &LuaRuntime::evaluationCompleted);
 
@@ -58,7 +56,7 @@ private slots:
     }
 
     context("when given an expression returning nil");
-    xit("emits evaluationCompleted with the result") {
+    it("emits evaluationCompleted with the result") {
       auto lua = LuaRuntime::instance();
       QSignalSpy evaluationCompletedSpy(lua, &LuaRuntime::evaluationCompleted);
 
@@ -71,9 +69,9 @@ private slots:
     }
   }
 
-  void testQueueTask() {
+  void test_queue_task() {
     context("when task is queued");
-    xit("evaluates task asynchronously") {
+    it("evaluates task asynchronously") {
       auto lua = LuaRuntime::instance();
       bool wasTaskCalled = false;
 
@@ -85,44 +83,46 @@ private slots:
     }
   }
 
-  void testSanityCheckUV() {
+  void test_sanity_check_uv_timer() {
     context("when a 1 second timer is set");
-    xit("calls callback after 1 second") {
+    it("calls callback after 1 second") {
       auto lua = LuaRuntime::instance();
       QSignalSpy evaluationCompletedSpy(lua, &LuaRuntime::evaluationCompleted);
 
       lua->evaluate(R"(
-        _G.wasTimerCalled = false;
+        _G.was_timer_called = false;
         local timer = uv.new_timer();
         timer:start(1000, 0, function()
-          _G.wasTimerCalled = true;
+          _G.was_timer_called = true;
           timer:close()
         end)
       )");
       QVERIFY(evaluationCompletedSpy.wait());
-      QVERIFY(NOT lua->evaluateSync("return _G.wasTimerCalled").toBool());
+      QVERIFY(NOT lua->evaluateSync("return _G.was_timer_called").toBool());
 
       QVERIFY(QTest::qWaitFor([&lua]() {
-        return lua->evaluateSync("return _G.wasTimerCalled").toBool();
+        return lua->evaluateSync("return _G.was_timer_called").toBool();
       }));
     }
   }
 
-  void testSanityCheckUVSpawn() {
-    it("emits evaluationCompleted with the result") {
+  void test_sanity_check_uv_spawn() {
+    it("calls exit callback when process exists") {
       auto lua = LuaRuntime::instance();
       QSignalSpy evaluationCompletedSpy(lua, &LuaRuntime::evaluationCompleted);
 
       lua->evaluate(R"(
-        _G.wasTimerCalled = false;
-        print('--------------- called')
-        local handle, pid = uv.spawn('ls', { args = {} }, function(code)
-          print('--------------- called')
-          print('DONE', code)
+        _G.spawn_exit_code = -1;
+        local handle, pid = uv.spawn('ls', {}, function(code)
+          _G.spawn_exit_code = code;
         end)
       )");
       QVERIFY(evaluationCompletedSpy.wait());
-      std::this_thread::sleep_for(std::chrono::milliseconds(2000));
+      QCOMPARE(lua->evaluateSync("return _G.spawn_exit_code").toInt(), -1);
+
+      QVERIFY(QTest::qWaitFor([&lua]() {
+        return 0 == lua->evaluateSync("return _G.spawn_exit_code").toInt();
+      }));
     }
   }
 };
diff --git a/spec/WebViewStackSpec.cpp b/spec/WebViewStackSpec.cpp
index 040fb8f..0cd58ed 100644
--- a/spec/WebViewStackSpec.cpp
+++ b/spec/WebViewStackSpec.cpp
@@ -18,7 +18,7 @@ class WebViewStackSpec : public QObject {
   };
 
 private slots:
-  void testInitialState() {
+  void test_initial_state() {
     context("when initialized");
     it("opens a single tab") {
       Configuration configuration;
@@ -30,7 +30,7 @@ private slots:
     }
   }
 
-  void testOpenUrl() {
+  void test_open_url() {
     context("when openUrl is called without an open type");
     it("replaces current tab url with newtab url") {
       Configuration configuration;
@@ -79,7 +79,7 @@ private slots:
     }
   }
 
-  void testNextNavigation() {
+  void test_next_navigation() {
     context("when nextWebView is called");
     context("- and there is only 1 tab");
     it("does nothing") {
@@ -122,7 +122,7 @@ private slots:
     }
   }
 
-  void testPreviousNavigation() {
+  void test_previous_navigation() {
     context("when previousWebView is called");
     context("- and there is only 1 tab");
     it("does nothing") {
@@ -165,7 +165,7 @@ private slots:
     }
   }
 
-  void testCloseWebView() {
+  void test_close_web_view() {
     context("when closeWebView is called");
     context("- with out of bounds index");
     it("does nothing") {
@@ -241,7 +241,7 @@ private slots:
     }
   }
 
-  void testNewWindowRequestSignal() {
+  void test_new_window_request_signal() {
     context("when webview emits a newWindowRequested signal");
     context("- of type new tab");
     it("opens a new web view and focusses it") {
diff --git a/src/AsyncEventLoop.cpp b/src/AsyncEventLoop.cpp
index 6c6333f..230d891 100644
--- a/src/AsyncEventLoop.cpp
+++ b/src/AsyncEventLoop.cpp
@@ -3,6 +3,7 @@
 #include <mutex>
 #include <queue>
 #include <thread>
+#include <uv.h>
 
 #include "AsyncEventLoop.hpp"
 
@@ -53,32 +54,36 @@ AsyncEventLoop::~AsyncEventLoop() {
     return;
   isLoopRunning = false;
 
-  // Wake it up. Stab it to death. (clear async queue)
-  wake();
-  std::this_thread::sleep_for(std::chrono::milliseconds(100));
-  {
-    std::lock_guard<std::mutex> lock(tasksQueueMutex);
-    std::queue<std::function<void()>>().swap(tasksQueue);
-  }
+  flushTasks();
+  std::this_thread::sleep_for(std::chrono::milliseconds(50));
   uv_stop(loop);
 
   // Close all handles
   AsyncEventLoop::closeHandle(reinterpret_cast<uv_handle_t *>(&asyncHandle));
   uv_walk(loop, AsyncEventLoop::closeHandle, nullptr);
-  while (uv_run(loop, UV_RUN_NOWAIT) != 0)
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-  // TODO: Fix pending handler case (setTimeout(100) wait(20) close() -> error)
 
   if (loopThread.joinable())
     loopThread.join();
 
-  while (uv_loop_close(loop))
-    uv_run(loop, UV_RUN_DEFAULT);
+  // Close loop
+  while (uv_loop_close(loop) == EBUSY)
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+  uv_run(loop, UV_RUN_DEFAULT);
+
+  if (uv_loop_alive(loop))
+    qDebug() << "WARNING: Loop still has active handles!";
 
   free(loop);
   loop = nullptr;
 }
 
+void AsyncEventLoop::flushTasks() {
+  std::lock_guard<std::mutex> lock(tasksQueueMutex);
+  std::queue<std::function<void()>>().swap(tasksQueue);
+  wake();
+}
+
 void AsyncEventLoop::asyncHandleCallback(uv_async_t *handle) {
   auto *runtime = static_cast<AsyncEventLoop *>(handle->data);
   runtime->processTasks();
diff --git a/src/AsyncEventLoop.hpp b/src/AsyncEventLoop.hpp
index 5718909..8082dfa 100644
--- a/src/AsyncEventLoop.hpp
+++ b/src/AsyncEventLoop.hpp
@@ -33,6 +33,9 @@ protected:
   static void closeHandle(uv_handle_t *handle, void *arg = nullptr);
 
 private:
+  void flushTasks();
+
+private:
   uv_loop_t *loop;
   std::thread loopThread;
   uv_async_t asyncHandle;