Issue #4692 - Move responsibility for engine reference from tasks to scheduler

src/FileSystemJsObject.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 15 matching lines @@
 #include "JsEngineTransition.h"
 #include "Scheduler.h"
 #include "Utils.h"
 #include "Value.h"
 
 using namespace AdblockPlus;
 
 namespace
 {
   class ReadTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal* engine;
     std::string path;
     V8PersistentNG<v8::Function> callbackFunction;
 
   public:
     ReadTask(JsEngineInternal* engine, const std::string& path,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path),
+      : engine(engine), path(path),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine = ToInternal(jsEngine);
       std::string content;
       std::string error;
       // Read operation is long-lived. Do not lock engine during it.
       try
       {
         std::shared_ptr<std::istream> stream = engine->GetFileSystem()->Read(path);
         content = Utils::Slurp(*stream);
       }
       catch (std::exception& e)
       {
         error = e.what();
       }
       catch (...)
       {
         error = "Unknown error while reading from " + path;
       }
       // Call back with the results of the read operation
       V8ExecutionScope scope(engine);
       auto callbackArguments = v8::Object::New();
       callbackArguments->Set(engine->ToV8String("content"), engine->ToV8String(content));
       callbackArguments->Set(engine->ToV8String("error"), engine->ToV8String(error));
       auto args = AllocatedArray<v8::Local<v8::Value>>(1);
       args[0] = callbackArguments;
       engine->ApplyFunction(callbackFunction.Get(engine->GetIsolate()), std::move(args));
     }
   };
 
   class WriteTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal* engine;
     std::string path;
     std::string content;
     V8PersistentNG<v8::Function> callbackFunction;
   public:
     WriteTask(JsEngineInternal* engine,
       const std::string& path, const std::string& content,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path), content(content),
+      : engine(engine), path(path), content(content),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine = ToInternal(jsEngine);
       std::string error;
       // Write operation is long-lived. Do not lock engine during it.
       try
       {
         std::shared_ptr<std::iostream> stream(new std::stringstream);
         *stream << content;
         engine->GetFileSystem()->Write(path, stream);
       }
       catch (std::exception& e)
       {
         error = e.what();
       }
       catch (...)
       {
         error = "Unknown error while writing to " + path;
       }
       // Apply the callback function
       V8ExecutionScope scope(engine);
       auto args = AllocatedArray<v8::Local<v8::Value>>(1);
       args[0] = engine->ToV8String(error);
       engine->ApplyFunction(callbackFunction.Get(engine->GetIsolate()), std::move(args));
     }
   };
 
   class MoveTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal* engine;
     std::string fromPath;
     std::string toPath;
     V8PersistentNG<v8::Function> callbackFunction;
 
   public:
     MoveTask(JsEngineInternal* engine, const std::string& fromPath, const std::string& toPath,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), fromPath(fromPath), toPath(toPath), 
+      : engine(engine), fromPath(fromPath), toPath(toPath),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine(ToInternal(jsEngine));
       std::string error;
       try
       {
         engine->GetFileSystem()->Move(fromPath, toPath);
       }
       catch (std::exception& e)
       {
         error = e.what();
       }
       catch (...)
       {
         error = "Unknown error while moving " + fromPath + " to " + toPath;
       }
       V8ExecutionScope scope(engine);
       auto args = AllocatedArray<v8::Local<v8::Value>>(1);
       args[0] = engine->ToV8String(error);
       engine->ApplyFunction(callbackFunction.Get(engine->GetIsolate()), std::move(args));
     }
   };
 
   class RemoveTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal *engine;
     std::string path;
     V8PersistentNG<v8::Function> callbackFunction;
 
   public:
     RemoveTask(JsEngineInternal *engine, const std::string& path,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path),
+      : engine(engine), path(path),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine(ToInternal(jsEngine));
       std::string error;
       try
       {
         engine->GetFileSystem()->Remove(path);
       }
       catch (std::exception& e)
       {
         error = e.what();
       }
       catch (...)
       {
         error = "Unknown error while removing " + path;
       }
       V8ExecutionScope scope(engine);
       auto args = AllocatedArray<v8::Local<v8::Value>>(1);
       args[0] = engine->ToV8String(error);
       engine->ApplyFunction(callbackFunction.Get(engine->GetIsolate()), std::move(args));
     }
   };
 
   class StatTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal *engine;
     std::string path;
     V8PersistentNG<v8::Function> callbackFunction;
 
   public:
     StatTask(JsEngineInternal* engine, const std::string& path,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path),
+      : engine(engine), path(path),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine(ToInternal(jsEngine));
       std::string error;
       FileSystem::StatResult statResult;
       try
       {
         statResult = engine->GetFileSystem()->Stat(path);
       }
       catch (std::exception& e)
       {
         error = e.what();
       }
@@ skipping 11 matching lines @@
       auto args = AllocatedArray<v8::Local<v8::Value>>(1);
       args[0] = callbackArgument;
       engine->ApplyFunction(callbackFunction.Get(engine->GetIsolate()), std::move(args));
     }
   };
 }
 
 v8::Handle<v8::Value> ReadCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<ReadTask> readTask;
   try
   {
     if (arguments.Length() != 2)
     {
       throw std::runtime_error("_fileSystem.read requires 2 parameters");
     }
     bool argumentIsString;
     std::string firstArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
       throw std::runtime_error("First argument to _fileSystem.read must be a string");
     }
     if (!arguments[1]->IsFunction())
     {
       throw std::runtime_error("Second argument to _fileSystem.read must be a function");
     }
-    readTask = std::make_shared<ReadTask>(engine, firstArgument, 
+    ReadTask readTask(engine, firstArgument,
       V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
+    // Run the task
+    engine->ScheduleTask(std::move(readTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  engine->Schedule(AdblockPlus::MakeHeapFunction(readTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> WriteCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<WriteTask> writeTask;
   try
   {
-    v8::Isolate* isolate = arguments.GetIsolate();
-
     if (arguments.Length() != 3)
     {
       throw std::exception("_fileSystem.write requires 3 parameters");
     }
     bool argumentIsString;
     std::string firstArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
       throw std::runtime_error("First argument to _fileSystem.write must be a string");
     }
     std::string secondArgument;
     std::tie(argumentIsString, secondArgument) = ConvertString(arguments[1]);
     if (!argumentIsString)
     {
       throw std::runtime_error("Second argument to _fileSystem.write must be a string");
     }
     if (!arguments[2]->IsFunction())
     {
       throw std::runtime_error("Third argument to _fileSystem.write must be a function");
     }
-    writeTask = std::make_shared<WriteTask>(engine, firstArgument, secondArgument, 
+    // Run the task
+    WriteTask writeTask(engine, firstArgument, secondArgument,
       V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[2])));
+    engine->ScheduleTask(std::move(writeTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  engine->Schedule(AdblockPlus::MakeHeapFunction(writeTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> MoveCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<MoveTask> moveTask;
-  std::string firstArgument, secondArgument;
   try
   {
     if (arguments.Length() != 3)
     {
       throw std::runtime_error("_fileSystem.move requires 3 parameters");
     }
     bool argumentIsString;
+    std::string firstArgument, secondArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
       throw std::runtime_error("First argument to _fileSystem.write must be a string");
     }
     std::tie(argumentIsString, secondArgument) = ConvertString(arguments[1]);
     if (!argumentIsString)
     {
       throw std::runtime_error("Second argument to _fileSystem.write must be a string");
     }
     if (!arguments[2]->IsFunction())
     {
       throw std::runtime_error("Third argument to _fileSystem.move must be a function");
     }
+    // Run the task
+    MoveTask moveTask(engine, firstArgument, secondArgument,
+      V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[2])));
+    engine->ScheduleTask(std::move(moveTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  moveTask = std::make_shared<MoveTask>(engine, firstArgument, secondArgument, 
-    V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[2])));
-  engine->Schedule(AdblockPlus::MakeHeapFunction(moveTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> RemoveCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<RemoveTask> removeTask;
-  std::string firstArgument;
   try
   {
     if (arguments.Length() != 2)
     {
       throw std::runtime_error("_fileSystem.remove requires 2 parameters");
     }
     bool argumentIsString;
+    std::string firstArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
       throw std::runtime_error("First argument to _fileSystem.remove must be a string");
     }
     if (!arguments[1]->IsFunction())
     {
       throw std::runtime_error("Second argument to _fileSystem.remove must be a function");
     }
+    // Run the task
+    RemoveTask removeTask(engine, firstArgument,
+      V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
+    engine->ScheduleTask(std::move(removeTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  removeTask = std::make_shared<RemoveTask>(engine, firstArgument,
-    V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
-  engine->Schedule(AdblockPlus::MakeHeapFunction(removeTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> StatCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<StatTask> statTask;
-  std::string firstArgument;
   try
   {
     if (arguments.Length() != 2)
     {
       throw std::runtime_error("_fileSystem.stat requires 2 parameters");
     }
     bool argumentIsString;
+    std::string firstArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
       throw std::runtime_error("First argument to _fileSystem.stat must be a string");
     }
     if (!arguments[1]->IsFunction())
     {
       throw std::runtime_error("Second argument to _fileSystem.stat must be a function");
     }
+    // Run the task
+    StatTask statTask(engine, firstArgument,
+      V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
+    engine->ScheduleTask(std::move(statTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  statTask = std::make_shared<StatTask>(engine, firstArgument,
-    V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
-  engine->Schedule(AdblockPlus::MakeHeapFunction(statTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> ResolveCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
   std::string firstArgument;
   try
   {
     if (arguments.Length() != 1)
     {
       throw std::runtime_error("_fileSystem.resolve requires 1 parameter");
     }
     bool argumentIsString;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
       throw std::runtime_error("First argument to _fileSystem.write must be a string");
     }
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
   /*
    * Make sure to perform long-lived file system operation with engine unlocked.
    */
   std::string resolved = engine->GetFileSystem()->Resolve(firstArgument);
   return engine->ToV8String(resolved);
 }