Issue 3594 - remove circular references JsEngine-JsValue-JsEngine

src/Utils.cpp

Index: src/Utils.cpp
diff --git a/src/Utils.cpp b/src/Utils.cpp
index 1020ec5b6334a1bd293cdb3ef353fd858ae8d633..6501e069c86a09d94e5ac7b816b24884f1204ba2 100644
--- a/src/Utils.cpp
+++ b/src/Utils.cpp
@@ -24,6 +24,7 @@
 #endif
 
 #include "Utils.h"
+#include <AdblockPlus/JsEngine.h>
 
 using namespace AdblockPlus;
 
@@ -104,3 +105,10 @@ std::wstring Utils::CanonizeUrl(const std::wstring& url)
 }
 #endif
 
+std::shared_ptr<JsEngine> Utils::lockJsEngine(const std::weak_ptr<JsEngine>& jsEngine)
+{
+  auto retValue = jsEngine.lock();
+  if (!retValue)
+    throw AdblockPlus::JsEngine::JsEngineNotAvailableException();
+  return retValue;
+}

[Eric, 2016/11/28 14:21:14]

This function doesn't do what its name indicates that it does. The lifespan of
the shared_ptr `retValue` only lasts the duration of the function body. After
the function returns, the engine argument has no more strong references than it
did at function entry. So there's no actually locking except while the function
is executing.

It's not that checking whether the engine exists behind the pointer isn't
valuable. But such a function shouldn't be named "lock" or anything like it. And
it would be more efficient just to call 'expired()' on the weak_ptr.

See comments where this is used for more.

[sergei, 2016/11/29 10:45:19]

That's just wrong.