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.
Showing the changes to libraries/script-engine/src/ScriptEngine.cpp (1 of 1 file). Show all
Unified · Split
| @@ -1639,22 +1639,24 @@ QVariantMap ScriptEngine::fetchModuleSource(const QString& modulePath, const boo · 1638 unmodified lines | |||
| 1639 | loader->start(MAX_RETRIES); | 1639 | loader->start(MAX_RETRIES); |
| 1640 | 1640 | ||
| 1641 | if (!loader->isFinished()) { | 1641 | if (!loader->isFinished()) { |
| 1642 | QTimer monitor; | 1642 | // This lambda can get called AFTER this local scope has completed. |
| 1643 | QEventLoop loop; | 1643 | // This is why we pass smart ptrs to the lambda instead of references to local variables. |
| 1644 | QObject::connect(loader, &BatchLoader::finished, this, [&monitor, &loop]{ | 1644 | auto monitor = std::make_shared<QTimer>(); |
| 1645 | monitor.stop(); | 1645 | auto loop = std::make_shared<QEventLoop>(); |
| 1646 | loop.quit(); | 1646 | QObject::connect(loader, &BatchLoader::finished, this, [monitor, loop] { |
| 1647 | monitor->stop(); | ||
| 1648 | loop->quit(); | ||
| 1647 | }); | 1649 | }); |
| 1648 | 1650 | ||
| 1649 | // this helps detect the case where stop() is invoked during the download | 1651 | // this helps detect the case where stop() is invoked during the download |
| 1650 | // but not seen in time to abort processing in onload()... | 1652 | // but not seen in time to abort processing in onload()... |
| 1651 | connect(&monitor, &QTimer::timeout, this, [this, &loop]{ | 1653 | connect(monitor.get(), &QTimer::timeout, this, [this, loop] { |
| 1652 | if (isStopping()) { | 1654 | if (isStopping()) { |
| 1653 | loop.exit(-1); | 1655 | loop->exit(-1); |
| 1654 | } | 1656 | } |
| 1655 | }); | 1657 | }); |
| 1656 | monitor.start(500); | 1658 | monitor->start(500); |
| 1657 | loop.exec(); | 1659 | loop->exec(); |
| 1658 | } | 1660 | } |
| 1659 | loader->deleteLater(); | 1661 | loader->deleteLater(); |
| 1660 | return req; | 1662 | return req; |