Issue #2230, #3391 - Load filters on "download begin" event

src/plugin/DetachedInitialization.h

Index: src/plugin/DetachedInitialization.h
===================================================================
--- a/src/plugin/DetachedInitialization.h
+++ b/src/plugin/DetachedInitialization.h
@@ -25,27 +25,19 @@
 #include <condition_variable>
 
 /**
- * A detached initializer for static class members
+ * Initialization performed in a detached thread
  *
- * Detached initializers use a separate thread to avoid delays at instantiation.

[sergei, 2016/01/29 10:03:20]

This line is OK.

- * Variables statically initialized in a DLL cannot, however, contain threads.
- * Thus such initialization must occur in the constructor at the earliest.

[sergei, 2016/01/29 10:03:20]

These two lines are not related to this file.

- *

[sergei, 2016/01/29 10:03:20]

It's important to say in the description whether the class is thread-safe, thus
whether it's supposed that all methods may be called "simultaneously" from
several threads.

BTW, what happens if somebody calls SpawnInitializer() and EnsureInitialized()
"simultaneously"? If we cannot spawn a thread (I hope it's very unlikely) then
the waiter will wait forever, right?

- * This kind of initialization acts shares much in common with a singleton.
- * Rather than constructing a singleton, instead we execute an initialization function.
+ * This kind of initialization shares much in common with a singleton.
+ * Rather than constructing a singleton, instead we execute an initialization function exactly once.

[sergei, 2016/01/29 10:03:19]

There is nothing about singleton.
"Detached initializers use a separate thread to avoid delays at instantiation."
looks completely sufficient.

  * The multiprocessing and synchronization issues, however, are essentially identical.

[sergei, 2016/01/29 10:03:20]

This line is not related to this file.

- *
- * The designated usage of this class:
- * 1) Define a static class member 'x' of this type with an initializer function.
- * 2) Call 'x.SpawnInitializer()' in the constructors to run the initializer.
- * 3) Call 'x.EnsureInitialized()' before relying upon any of the side effects of the initializer.
- *
  * The implementation acts much like a multithreaded singleton initialization using a double-checked lock.
  * See https://en.wikipedia.org/wiki/Double-checked_locking and http://preshing.com/20130922/acquire-and-release-fences/

[sergei, 2016/01/29 10:03:20]

I would say that semantically it looks completely differently. One can notice
some similarity in the implementation but I would say that comment is rather
confusing.

  * The flag members of this class enable fast-return paths, that is, a return without obtaining a mutex lock.

[sergei, 2016/01/29 10:03:19]

I don't think we need this line.

  *
- * \tparam F
- *   The initializer function
+ * Designated usage:
+ * 1) Derive from this class, defining the factory Initializer().

[sergei, 2016/01/29 10:03:19]

I am afraid, I disagree with such approach (even beside the virtual
initializer).
The main issue is that it's very easy to forget to call `EnsureInitialized()`
from the destructor of the class (as it happens here) what leads to the race
condition, the derived class with all members can be already destroyed but the
initializing function using them is still running.

+ * 2) Call 'SpawnInitializer()' in the constructors.
+ * 3) Call 'EnsureInitialized()' before relying upon any of the side effects of the initializer.
  *
  * \par Serialization Guarantees
  * The visible effect of these function returns are serialized: 
@@ -53,7 +45,6 @@
  *   - ThreadMain() before EnsureInitialized()
  *   - If ThreadMain() started, ThreadMain() before destructor
  */
-template<void (&F)()>
 class DetachedInitializer
 {
   // "protected" declaration allows white-box subclass for testing
@@ -103,17 +94,32 @@
   {
     return hasInitializerStarted.load(std::memory_order_acquire);
   }
+  
+  /**
+   * Type of initializer objects
+   *
+   * Note that the underlying function returns 'void'.
+   * This class refuses responsibility for specific error semantics.
+   * If the initializer thread throws, then so will EnsureInitialized().

[sergei, 2016/01/29 10:03:19]

Only this line is important, the rest about void and specific error semantics
looks redundant.

+   * Anything beyond this is the responsibility of the owner.
+   */
+  typedef std::function<void()> InitializerType;
 
   /**
-   * Main function for thread
+   * Factory function for initializers
    */
-  void ThreadMain()
+  virtual InitializerType Initializer() = 0;
+
+  /**
+   * Main function for initialization thread
+   */
+  void ThreadMain(InitializerType f)
   {
     try
     {
-      F();
+      f();
     }
-    catch (std::exception& e)
+    catch (...)
     {
       // Assert initializer threw an exception
       initializerException = std::current_exception();
@@ -217,7 +223,7 @@
          * If it does throw, the initialization-started flag won't be set.
          * An exception here is thus a soft error, since a future invocation might succeed.
          */
-        std::thread th([this]() -> void { ThreadMain(); });
+        std::thread th(std::bind([this](InitializerType f) -> void { ThreadMain(f); }, Initializer()));
         // Memory fence ensures store ends before any other load or store
         hasInitializerStarted.store(true, std::memory_order_release);
         th.detach(); // Won't throw because the thread is both valid and joinable
@@ -255,4 +261,19 @@
   }
 };
 
+/**
+ * Detached initializers use a separate thread to avoid delays at instantiation.
+ * The constructors of variables statically initialized in a DLL cannot, however, contain threads.
+ * Thus such initialization must (generally) occur in the constructor.
+ */
+template<void(&F)()>
+class DetachedInitializerStaticFunction
+  : public DetachedInitializer
+{
+  InitializerType Initializer() override
+  {
+    return InitializerType([] { F(); });
+  }
+};
+
 #endif // _DETACHED_INITIALIZATION_H_