Merge remote-tracking branch 'origin/develop' into ref-cleanup

Conflicts:
	src/autotesting/AutowiringEnclosure.cpp

autowiring/AutowiringEnclosure.h

@@ -1,9 +1,12 @@
 // Copyright (C) 2012-2014 Leap Motion, Inc. All rights reserved.
 #pragma once
 #include <autowiring/autowiring.h>
-#include <gtest/gtest.h>
 #include MEMORY_HEADER
 
+#ifndef GTEST_INCLUDE_GTEST_GTEST_H_
+#error Please include your version of gtest.h before including the autowiring enclosure
+#endif
+
 /// <summary>
 /// General purpose exception filter, used for tracking exceptions thrown from unit test CoreThreads
 /// </summary>
@@ -35,6 +38,14 @@ class AutowiringEnclosureExceptionFilter:
   }
 };
 
+struct TestInfoProxy {
+  TestInfoProxy(const testing::TestInfo& info) :
+    m_info(info)
+  {}
+
+  const testing::TestInfo& m_info;
+};
+
 /// <summary>
 /// Provides a test fixture which ensures proper cleanup of a created subcontext
 /// </summary>
@@ -53,6 +64,54 @@ class AutowiringEnclosure:
 
 public:
   // Base overrides:
-  void OnTestStart(const testing::TestInfo& info) override;
-  void OnTestEnd(const testing::TestInfo& info) override;
+  void OnTestStart(const testing::TestInfo& info) override {
+    AutoRequired<AutowiringEnclosureExceptionFilter> filter;
+
+    // The context proper.  This is automatically assigned as the current
+    // context when SetUp is invoked.
+    AutoCreateContext create;
+    create->Construct<TestInfoProxy>(info);
+
+    // Add exception filter in this context:
+    create->Inject<AutowiringEnclosureExceptionFilter>();
+
+    // Now make it current and let the test run:
+    create->SetCurrent();
+  }
+
+  void OnTestEnd(const testing::TestInfo& info) override {
+    // Verify we can grab the test case back out and that the pointer is correct:
+    Autowired<TestInfoProxy> ti;
+    Autowired<AutowiringEnclosureExceptionFilter> ecef;
+    auto ctxt = ecef ? ecef->GetContext() : nullptr;
+
+    // Unconditionally reset the global context as the current context
+    AutoGlobalContext()->SetCurrent();
+
+    // Global initialization tests are special, we don't bother checking closure principle on them:
+    if(!strcmp("GlobalInitTest", info.test_case_name()))
+      return;
+
+    // Need to make sure we got back our exception filter before continuing:
+    ASSERT_TRUE(ecef.IsAutowired()) << "Failed to find the enclosed context exception filter; unit test may have incorrectly reset the enclosing context before returning";
+
+    // Now try to tear down the test context enclosure:
+    ctxt->SignalShutdown();
+
+    // Do not allow teardown to take more than 250 milliseconds or so
+    if(!ctxt->Wait(std::chrono::milliseconds(250))) {
+      // Critical error--took too long to tear down
+      assert(false);
+    }
+
+    static const char sc_autothrow [] = "AUTOTHROW_";
+    if(!strncmp(sc_autothrow, info.name(), sizeof(sc_autothrow) - 1))
+      // Throw expected, end here
+      return;
+
+    // If an exception occurred somewhere, report it:
+    ASSERT_FALSE(ecef->m_excepted)
+      << "An unhandled exception occurred in this context" << std::endl
+      << "[" << (ecef->m_ti ? ecef->m_ti->name() : "unknown") << "] " << ecef->m_what;
+  }
 };

autowiring/ObjectPool.h

@@ -95,7 +95,7 @@ class ObjectPool
   // time the ClearCachedEntities method is called, and causes entities which might be trying
   // to return to the pool to instead free themselves.
   size_t m_poolVersion;
-  std::vector<std::unique_ptr<T>> m_objs;
+  std::vector<T*> m_objs;
 
   size_t m_maxPooled;
   size_t m_limit;
@@ -129,19 +129,19 @@ class ObjectPool
         // Finalize object before destruction or return to pool
         final(*ptr);
 
-        // Default behavior will be to destroy the pointer
-        std::unique_ptr<T> unique(ptr);
-
-        // Hold the lock next, in order to ensure that destruction happens after the monitor
-        // lock is released.
-        std::lock_guard<std::mutex> lk(*monitor);
-
-        if(monitor->IsAbandoned())
-          // Nothing we can do, monitor object abandoned already, just destroy the object
-          return;
-
-        // Pass control to the Return method
-        static_cast<ObjectPool<T>*>(monitor->GetOwner())->Return(poolVersion, unique);
+        bool inPool = false;
+        {
+          // Obtain lock before deciding whether to delete or return to pool
+          std::lock_guard<std::mutex> lk(*monitor);
+          if(!monitor->IsAbandoned()) {
+            // Attempt to return object to pool
+            inPool = static_cast<ObjectPool<T>*>(monitor->GetOwner())->ReturnUnsafe(poolVersion, ptr);
+          }
+        }
+        if (!inPool) {
+          // Destroy returning object outside of lock
+          delete ptr;
+        }
       }
     );
 
@@ -152,26 +152,30 @@ class ObjectPool
     return retVal;
   }
 
-  void Return(size_t poolVersion, std::unique_ptr<T>& unique) {
+  bool ReturnUnsafe(size_t poolVersion, T* ptr) {
     // ASSERT: Object has already been finalized
     // Always decrement the count when an object is no longer outstanding
     assert(m_outstanding);
     m_outstanding--;
 
+    bool inPool = false;
     if(
       // Pool versions have to match, or the object should be dumped
       poolVersion == m_poolVersion &&
 
       // Object pool needs to be capable of accepting another object as an input
       m_objs.size() < m_maxPooled
-    ) {
+       ) {
       // Return the object to the pool:
-      m_objs.emplace_back(std::move(unique));
+      m_objs.push_back(ptr);
+      inPool = true;
     }
 
     // If the new outstanding count is less than or equal to the limit, wake up any waiters:
     if(m_outstanding <= m_limit)
       m_setCondition.notify_all();
+
+    return inPool;
   }
 
   /// <summary>
@@ -196,8 +200,8 @@ class ObjectPool
     }
 
     // Transition from pooled to issued:
-    std::shared_ptr<T> iObj = Wrap(m_objs.back().release()); // Takes ownership
-    m_objs.pop_back(); // Removes non-referencing object
+    std::shared_ptr<T> iObj = Wrap(m_objs.back()); // Takes ownership
+    m_objs.pop_back(); // Remove unsafe reference
     return iObj;
   }
 
@@ -228,14 +232,11 @@ class ObjectPool
   /// prior to this call.
   /// </remarks>
   void ClearCachedEntities(void) {
-    // Declare this first, so it's freed last:
-    std::vector<std::unique_ptr<T>> objs;
-
-    // Default destructor is using in object pool, so it is safe to destroy all
-    // in pool while holding lock.
     std::lock_guard<std::mutex> lk(*m_monitor);
-    m_poolVersion++;
+    for (T* obj : m_objs)
+      delete obj;
     m_objs.clear();
+    m_poolVersion++;
   }
 
   /// <summary>
@@ -257,6 +258,7 @@ class ObjectPool
         return;
 
       // Remove unique pointer
+      delete m_objs.back();
       m_objs.pop_back();
     }
   }

autowiring/atomic_object.h

@@ -1,5 +1,6 @@
 // Copyright (c) 2010 - 2014 Leap Motion. All rights reserved. Proprietary and confidential.
 #pragma once
+#include MEMORY_HEADER
 #include MUTEX_HEADER
 
 #include "unlock_object.h"

contrib/C++11/memory.h

@@ -2,4 +2,4 @@
 #pragma once
 
 #include <memory>
-#include "contrib/C++11/make_unique.h"
+#include "make_unique.h"

contrib/C++11/memory_nostl11.h

@@ -1,9 +1,9 @@
 // Copyright (C) 2012-2014 Leap Motion, Inc. All rights reserved.
 #pragma once
 
-#include "C++11/boost_shared_ptr.h"
-#include "C++11/unique_ptr.h"
-#include "C++11/make_unique.h"
+#include "boost_shared_ptr.h"
+#include "unique_ptr.h"
+#include "make_unique.h"
 
 #include STL_UNORDERED_SET
 template<typename T>

src/autonet/AutoNetServerImpl.cpp

@@ -1,6 +1,6 @@
 #include "stdafx.h"
 #include "AutoNetServerImpl.hpp"
-#include "Autowiring.h"
+#include "autowiring.h"
 #include "at_exit.h"
 #include "EventRegistry.h"
 #include "TypeRegistry.h"

src/autonet/AutoNetServerImpl.hpp

@@ -1,13 +1,18 @@
 #pragma once
 #include "AutoNetServer.h"
 #include "AutowiringEvents.h"
-#include <cctype>
 #include <json11/json11.hpp>
-#include <websocketpp/server.hpp>
-#include <websocketpp/config/asio_no_tls.hpp>
+#include <cctype>
 #include <map>
 #include <set>
 
+// Need to redefine this namespace so we don't create linker problems.  Such problems may be
+// created if our static library is imported by another project that uses an incompatible
+// version of websocketpp.
+#define websocketpp websocketpp_autonet
+#include <websocketpp/server.hpp>
+#include <websocketpp/config/asio_no_tls.hpp>
+
 struct TypeIdentifierBase;
 
 class AutoNetServerImpl:

src/autotesting/CMakeLists.txt

@@ -1,6 +1,5 @@
 set(AutoTesting_SOURCES
   AutowiringEnclosure.h
-  AutowiringEnclosure.cpp
   gtest-all-guard.h
   gtest-all-guard.cpp
 )

src/autotesting/gtest-all-guard.cpp

@@ -1,7 +1,7 @@
 // Copyright (C) 2012-2014 Leap Motion, Inc. All rights reserved.
 #include "stdafx.h"
-#include "AutowiringEnclosure.h"
 #include <gtest/gtest-all.cc>
+#include "AutowiringEnclosure.h"
 
 using namespace std;
 

src/autowiring/BasicThread.cpp

@@ -144,8 +144,10 @@ bool BasicThread::Start(std::shared_ptr<Object> outstanding) {
     m_state->m_stateCondition.notify_all();
   }
 
-  // Kick off a thread and return here
-  m_state->m_thisThread = std::thread(
+  // Place the new thread entity directly in the space where it goes to avoid
+  // any kind of races arising from asynchronous access to this space
+  m_state->m_thisThread.~thread();
+  new (&m_state->m_thisThread) std::thread(
     [this, outstanding] () mutable {
       this->DoRun(std::move(outstanding));
     }

src/autowiring/CoreThreadWin.cpp

@@ -41,7 +41,7 @@ void SetThreadName(DWORD dwThreadID, LPCSTR szThreadName)
 }
 
 void BasicThread::SetCurrentThreadName(void) const {
-  DWORD threadId = ::GetThreadId(static_cast<HANDLE>(m_state->m_thisThread.native_handle()));
+  DWORD threadId = ::GetThreadId(m_state->m_thisThread.native_handle());
   ::SetThreadName(threadId, m_name);
 }