From d2146e38810e3d012859325901cf0548d9264eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Thu, 22 Feb 2018 20:09:31 +0100 Subject: [PATCH 1/5] Fix shutdown of the worker thread pool. Shuts down the pool before terminating the threads to avoid accessing an event thread waiter slot that is not associated with a valid eventcore driver anymore. --- source/vibe/core/core.d | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/source/vibe/core/core.d b/source/vibe/core/core.d index 83bef51..998971e 100644 --- a/source/vibe/core/core.d +++ b/source/vibe/core/core.d @@ -254,6 +254,7 @@ void exitEventLoop(bool shutdown_all_threads = false) assert(s_eventLoopRunning || shutdown_all_threads, "Exiting event loop when none is running."); if (shutdown_all_threads) { () @trusted nothrow { + shutdownWorkerPool(); atomicStore(st_term, true); st_threadsSignal.emit(); } (); @@ -1349,13 +1350,8 @@ static ~this() } if (is_main_thread) { - shared(TaskPool) tpool; - synchronized (st_threadsMutex) swap(tpool, st_workerPool); - if (tpool) { - logDiagnostic("Main thread still waiting for worker threads."); - tpool.terminate(); - } logDiagnostic("Main thread exiting"); + shutdownWorkerPool(); } synchronized (st_threadsMutex) { @@ -1373,6 +1369,19 @@ static ~this() st_threadShutdownCondition.notifyAll(); } +private void shutdownWorkerPool() +nothrow { + shared(TaskPool) tpool; + + try synchronized (st_threadsMutex) swap(tpool, st_workerPool); + catch (Exception e) assert(false, e.msg); + + if (tpool) { + logDiagnostic("Still waiting for worker threads to exit."); + tpool.terminate(); + } +} + private void shutdownDriver() { if (ManualEvent.ms_threadEvent != EventID.init) { From 0ff503afa67ce13c70281dfd273b6366d1a2778b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Thu, 22 Feb 2018 15:53:44 +0100 Subject: [PATCH 2/5] Add test case for #58. --- tests/issue-58-task-already-scheduled.d | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/issue-58-task-already-scheduled.d diff --git a/tests/issue-58-task-already-scheduled.d b/tests/issue-58-task-already-scheduled.d new file mode 100644 index 0000000..2b99a38 --- /dev/null +++ b/tests/issue-58-task-already-scheduled.d @@ -0,0 +1,46 @@ +/+ dub.sdl: + name "tests" + dependency "vibe-core" path=".." ++/ +module tests; + +import vibe.core.sync; +import vibe.core.core; +import std.datetime; +import core.atomic; + +shared ManualEvent ev; +shared size_t counter; + +shared static this() +{ + ev = createSharedManualEvent(); +} + +void main() +{ + setTaskStackSize(64*1024); + + runTask({ + foreach (x; 0 .. 500) { + runWorkerTask(&worker); + } + }); + + setTimer(dur!"msecs"(10), { ev.emit(); }); + setTimer(dur!"seconds"(20), { assert(false, "Timers didn't fire within the time limit"); }); + + runApplication(); + + assert(atomicLoad(counter) == 500, "Event loop exited prematurely."); +} + +void worker() +{ + ev.wait(); + ev.emit(); + setTimer(dur!"seconds"(1), { + auto c = atomicOp!"+="(counter, 1); + if (c == 500) exitEventLoop(true); + }); +} From 2653c8c5e03276537695fe52377a0b474bb9c4d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Thu, 22 Feb 2018 16:14:26 +0100 Subject: [PATCH 3/5] Always remove task from queue when switching to it. Fixes #58. Previously the task was only removed if switchTo was called from another task. Now it also gets removed when switchTo (e.g. due to a runTask() call) gets called form outside of a task. --- source/vibe/core/task.d | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/source/vibe/core/task.d b/source/vibe/core/task.d index 1aa02f4..cb187ac 100644 --- a/source/vibe/core/task.d +++ b/source/vibe/core/task.d @@ -414,6 +414,8 @@ final package class TaskFiber : Fiber { } } + assert(!m_queue, "Fiber done but still scheduled to be resumed!?"); + // make the fiber available for the next task recycleFiber(this); } @@ -456,7 +458,7 @@ final package class TaskFiber : Fiber { try call(Fiber.Rethrow.no); catch (Exception e) assert(false, e.msg); } (); - } + } /** Blocks until the task has ended. */ @@ -779,21 +781,23 @@ package struct TaskScheduler { auto thisthr = thist ? thist.thread : () @trusted { return Thread.getThis(); } (); assert(t.thread is thisthr, "Cannot switch to a task that lives in a different thread."); + + auto tf = () @trusted { return t.taskFiber; } (); + if (tf.m_queue) { + debug (VibeTaskLog) logTrace("Task to switch to is already scheduled. Moving to front of queue."); + assert(tf.m_queue is &m_taskQueue, "Task is already enqueued, but not in the main task queue."); + m_taskQueue.remove(tf); + assert(!tf.m_queue, "Task removed from queue, but still has one set!?"); + } + if (thist == Task.init && defer == No.defer) { assert(TaskFiber.getThis().m_yieldLockCount == 0, "Cannot yield within an active yieldLock()!"); debug (VibeTaskLog) logTrace("switch to task from global context"); resumeTask(t); debug (VibeTaskLog) logTrace("task yielded control back to global context"); } else { - auto tf = () @trusted { return t.taskFiber; } (); auto thistf = () @trusted { return thist.taskFiber; } (); assert(!thistf || !thistf.m_queue, "Calling task is running, but scheduled to be resumed!?"); - if (tf.m_queue) { - debug (VibeTaskLog) logTrace("Task to switch to is already scheduled. Moving to front of queue."); - assert(tf.m_queue is &m_taskQueue, "Task is already enqueued, but not in the main task queue."); - m_taskQueue.remove(tf); - assert(!tf.m_queue, "Task removed from queue, but still has one set!?"); - } debug (VibeTaskLog) logDebugV("Switching tasks (%s already in queue)", m_taskQueue.length); if (defer) { @@ -878,6 +882,7 @@ package struct TaskScheduler { debug if (TaskFiber.ms_taskEventCallback) () @trusted { TaskFiber.ms_taskEventCallback(TaskEvent.yield, task); } (); () @trusted { Fiber.yield(); } (); debug if (TaskFiber.ms_taskEventCallback) () @trusted { TaskFiber.ms_taskEventCallback(TaskEvent.resume, task); } (); + assert(!task.m_fiber.m_queue, "Task is still scheduled after resumption."); } } From 58ae644c942d043c7adaf0d2de1ce5f15aaff7be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Fri, 23 Feb 2018 10:42:20 +0100 Subject: [PATCH 4/5] Fix AppVeyor LDC 1.7.0 download URL. --- appveyor.yml | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index aecde4b..a5f5cb5 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -78,11 +78,20 @@ install: } $env:toolchain = "msvc"; $version = $env:DVersion; - Invoke-WebRequest "https://github.com/ldc-developers/ldc/releases/download/v$($version)/ldc2-$($version)-win64-msvc.zip" -OutFile "c:\ldc.zip"; - echo "finished."; - pushd c:\\; - 7z x ldc.zip > $null; - popd; + if ([System.Version]$version -lt [System.Version]"1.7.0") { + Invoke-WebRequest "https://github.com/ldc-developers/ldc/releases/download/v$($version)/ldc2-$($version)-win64-msvc.zip" -OutFile "c:\ldc.zip"; + echo "finished."; + pushd c:\\; + 7z x ldc.zip > $null; + popd; + } + else { + Invoke-WebRequest "https://github.com/ldc-developers/ldc/releases/download/v$($version)/ldc2-$($version)-windows-multilib.7z" -OutFile "c:\ldc.7z"; + echo "finished."; + pushd c:\\; + 7z x ldc.7z > $null; + popd; + } } } - ps: SetUpDCompiler @@ -109,7 +118,11 @@ before_build: } elseif($env:DC -eq "ldc"){ $version = $env:DVersion; - $env:PATH += ";C:\ldc2-$($version)-win64-msvc\bin"; + if ([System.Version]$version -lt [System.Version]"1.7.0") { + $env:PATH += ";C:\ldc2-$($version)-win64-msvc\bin"; + } else { + $env:PATH += ";C:\ldc2-$($version)-windows-multilib\bin"; + } $env:DC = "ldc2"; } - ps: $env:compilersetup = "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall"; From f0bdaa07785f216d660f3372fc8260c57e45222d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Fri, 23 Feb 2018 15:49:49 +0100 Subject: [PATCH 5/5] Increase test timeout. --- tests/issue-58-task-already-scheduled.d | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/issue-58-task-already-scheduled.d b/tests/issue-58-task-already-scheduled.d index 2b99a38..80cc9f6 100644 --- a/tests/issue-58-task-already-scheduled.d +++ b/tests/issue-58-task-already-scheduled.d @@ -12,6 +12,8 @@ import core.atomic; shared ManualEvent ev; shared size_t counter; +enum ntasks = 500; + shared static this() { ev = createSharedManualEvent(); @@ -22,17 +24,16 @@ void main() setTaskStackSize(64*1024); runTask({ - foreach (x; 0 .. 500) { + foreach (x; 0 .. ntasks) runWorkerTask(&worker); - } }); setTimer(dur!"msecs"(10), { ev.emit(); }); - setTimer(dur!"seconds"(20), { assert(false, "Timers didn't fire within the time limit"); }); + setTimer(dur!"seconds"(60), { assert(false, "Timers didn't fire within the time limit"); }); runApplication(); - assert(atomicLoad(counter) == 500, "Event loop exited prematurely."); + assert(atomicLoad(counter) == ntasks, "Event loop exited prematurely."); } void worker() @@ -41,6 +42,6 @@ void worker() ev.emit(); setTimer(dur!"seconds"(1), { auto c = atomicOp!"+="(counter, 1); - if (c == 500) exitEventLoop(true); + if (c == ntasks) exitEventLoop(true); }); }