From ac6fcf092c52ab42d50c4d054bbaf512f305063d Mon Sep 17 00:00:00 2001
From: "Anthony J. Thibault" <tony@highfidelity.io>
Date: Wed, 27 Jun 2018 16:57:57 -0700
Subject: [PATCH] Fix memory corruption via lambdas that capture local
 variables by reference.

The lambdas in ScriptEngine::fetchModuleSource() were referencing local stack variables by reference.
This could lead to un-expected results including memory corruption.
To workaround this issue the QTimer and QEventLoop variables are allocated on the heap and held onto by a shared_ptr.
This shared_ptr is passed to the lambda.

This will not result in cycles and should result in the QTimer and QEventLoop being destroyed when the BatchLoader
object they are connected to is deleted.
---
 libraries/script-engine/src/ScriptEngine.cpp | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index f8c99b192f..99c02ba1f6 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -1639,22 +1639,24 @@ QVariantMap ScriptEngine::fetchModuleSource(const QString& modulePath, const boo
     loader->start(MAX_RETRIES);
 
     if (!loader->isFinished()) {
-        QTimer monitor;
-        QEventLoop loop;
-        QObject::connect(loader, &BatchLoader::finished, this, [&monitor, &loop]{
-            monitor.stop();
-            loop.quit();
+        // This lambda can get called AFTER this local scope has completed.
+        // This is why we pass smart ptrs to the lambda instead of references to local variables.
+        auto monitor = std::make_shared<QTimer>();
+        auto loop = std::make_shared<QEventLoop>();
+        QObject::connect(loader, &BatchLoader::finished, this, [monitor, loop] {
+            monitor->stop();
+            loop->quit();
         });
 
         // this helps detect the case where stop() is invoked during the download
         //  but not seen in time to abort processing in onload()...
-        connect(&monitor, &QTimer::timeout, this, [this, &loop]{
+        connect(monitor.get(), &QTimer::timeout, this, [this, loop] {
             if (isStopping()) {
-                loop.exit(-1);
+                loop->exit(-1);
             }
         });
-        monitor.start(500);
-        loop.exec();
+        monitor->start(500);
+        loop->exec();
     }
     loader->deleteLater();
     return req;