Move enter/exit context responsibility from PHP to join(), try #2

Move the responsibility of entering/exiting contexts from PHP to the
implementation of $wait_handle->join().

This eliminates possibility of weird situations, like contexts without
any running wait handle. This guarantees that asio_get_current() returns
null only if called completely out of asio framework and simplifies some
logic, such as getCurrentWaitHandleDepth().

hphp/runtime/ext/asio/asio_session.cpp

@@ -17,12 +17,16 @@
 
 #include <runtime/ext/ext_asio.h>
 #include <runtime/ext/asio/asio_session.h>
+#include <system/lib/systemlib.h>
 
 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 
 IMPLEMENT_THREAD_LOCAL_PROXY(AsioSession, false, AsioSession::s_current);
 
+namespace {
+  const context_idx_t MAX_CONTEXT_DEPTH = std::numeric_limits<context_idx_t>::max();
+}
 
 void AsioSession::Init() {
   s_current.set(new AsioSession());
@@ -31,21 +35,36 @@ void AsioSession::Init() {
 AsioSession::AsioSession() : m_contexts(), m_onFailedCallback(nullptr) {
 }
 
-uint16_t AsioSession::getCurrentWaitHandleDepth() {
-  // have context and it's running
-  if (!m_contexts.empty() && m_contexts.back()->isRunning()) {
-    return m_contexts.back()->getCurrent()->getDepth();
-  }
+void AsioSession::enterContext() {
+  assert(!isInContext() || getCurrentContext()->isRunning());
 
-  // the current context is not running, look at the upper context
-  // TODO: deprecate this once contexts are entered only by join()
-  if (m_contexts.size() >= 2) {
-    assert(m_contexts[m_contexts.size() - 2]->isRunning());
-    return m_contexts[m_contexts.size() - 2]->getCurrent()->getDepth();
+  if (UNLIKELY(getCurrentContextIdx() >= MAX_CONTEXT_DEPTH)) {
+    Object e(SystemLib::AllocInvalidOperationExceptionObject(
+      "Unable to enter asio context: too many contexts open"));
+    throw e;
   }
 
-  // we are the root
-  return 0;
+  m_contexts.push_back(new AsioContext());
+
+  assert(static_cast<context_idx_t>(m_contexts.size()) == m_contexts.size());
+  assert(isInContext());
+  assert(!getCurrentContext()->isRunning());
+}
+
+void AsioSession::exitContext() {
+  assert(isInContext());
+  assert(!getCurrentContext()->isRunning());
+
+  m_contexts.back()->exit(m_contexts.size());
+  delete m_contexts.back();
+  m_contexts.pop_back();
+
+  assert(!isInContext() || getCurrentContext()->isRunning());
+}
+
+uint16_t AsioSession::getCurrentWaitHandleDepth() {
+  assert(!isInContext() || getCurrentContext()->isRunning());
+  return isInContext() ? getCurrentWaitHandle()->getDepth() : 0;
 }
 
 void AsioSession::setOnFailedCallback(ObjectData* on_failed_callback) {

hphp/runtime/ext/asio/asio_session.h

@@ -35,18 +35,8 @@ class AsioSession {
     void operator delete(void* ptr) { smart_free(ptr); }
 
     // context
-    void enterContext() {
-      assert(!isInContext() || getCurrentContext()->isRunning());
-      m_contexts.push_back(new AsioContext());
-      assert(static_cast<context_idx_t>(m_contexts.size()) == m_contexts.size());
-    }
-
-    void exitContext() {
-      assert(isInContext());
-      m_contexts.back()->exit(m_contexts.size());
-      delete m_contexts.back();
-      m_contexts.pop_back();
-    }
+    void enterContext();
+    void exitContext();
 
     bool isInContext() {
       return !m_contexts.empty();
@@ -58,7 +48,8 @@ class AsioSession {
     }
 
     AsioContext* getCurrentContext() {
-      return m_contexts.empty() ? nullptr : m_contexts.back();
+      assert(isInContext());
+      return m_contexts.back();
     }
 
     context_idx_t getCurrentContextIdx() {
@@ -67,7 +58,8 @@ class AsioSession {
     }
 
     c_ContinuationWaitHandle* getCurrentWaitHandle() {
-      return m_contexts.empty() ? nullptr : m_contexts.back()->getCurrent();
+      assert(!isInContext() || getCurrentContext()->isRunning());
+      return isInContext() ? getCurrentContext()->getCurrent() : nullptr;
     }
 
     uint16_t getCurrentWaitHandleDepth();

hphp/runtime/ext/asio/static_exception_wait_handle.cpp

@@ -52,11 +52,6 @@ Object c_StaticExceptionWaitHandle::ti_create(const char* cls, CObjRef exception
   return wh;
 }
 
-const TypedValue* c_StaticExceptionWaitHandle::join() {
-  Object e(m_resultOrException.m_data.pobj);
-  throw e;
-}
-
 String c_StaticExceptionWaitHandle::getName() {
   return s_staticException;
 }

hphp/runtime/ext/asio/static_result_wait_handle.cpp

@@ -46,10 +46,6 @@ Object c_StaticResultWaitHandle::ti_create(const char* cls, CVarRef result) {
   return wh;
 }
 
-const TypedValue* c_StaticResultWaitHandle::join() {
-  return &m_resultOrException;
-}
-
 String c_StaticResultWaitHandle::getName() {
   return s_staticResult;
 }

hphp/runtime/ext/asio/wait_handle.cpp

@@ -48,7 +48,22 @@ void c_WaitHandle::t_import() {
 }
 
 Variant c_WaitHandle::t_join() {
-  return tvAsCVarRef(join());
+  if (!isFinished()) {
+    // run the full blown machinery
+    assert(dynamic_cast<c_WaitableWaitHandle*>(this));
+    static_cast<c_WaitableWaitHandle*>(this)->join();
+  }
+
+  assert(isFinished());
+
+  if (LIKELY(isSucceeded())) {
+    // succeeded? return result
+    return tvAsCVarRef(getResult());
+  } else {
+    // failed? throw exception
+    Object e(getException());
+    throw e;
+  }
 }
 
 bool c_WaitHandle::t_isfinished() {

hphp/runtime/ext/asio/waitable_wait_handle.cpp

@@ -156,49 +156,35 @@ c_BlockableWaitHandle* c_WaitableWaitHandle::getParentInContext(context_idx_t ct
   return parent;
 }
 
-const TypedValue* c_WaitableWaitHandle::join() {
-  switch (getState()) {
-    case STATE_NEW:
-      throw FatalErrorException(
-          "Invariant violation: uninitialized wait handle");
-    case STATE_SUCCEEDED:
-      return &m_resultOrException;
-    case STATE_FAILED:
-      Object e(m_resultOrException.m_data.pobj);
-      throw e;
-  }
-
+// throws on context depth level overflows and cross-context cycles
+void c_WaitableWaitHandle::join() {
   AsioSession* session = AsioSession::Get();
-  if (UNLIKELY(!session->isInContext())) {
-    Object e(SystemLib::AllocInvalidOperationExceptionObject(
-        "Unable to join wait handle: not in an asio context"));
-    throw e;
-  }
-  if (UNLIKELY(session->getCurrentContext()->isRunning())) {
-    Object e(SystemLib::AllocInvalidOperationExceptionObject(
-        "Unable to join wait handle: context busy, another join in progress"));
-    throw e;
-  }
 
-  // throws if cross-context cycle found
-  enterContext(session->getCurrentContextIdx());
-
-  // not finished, run queue
-  session->getCurrentContext()->runUntil(this);
-
-  switch (getState()) {
-    case STATE_NEW:
-      throw FatalErrorException(
-          "Invariant violation: uninitialized wait handle");
-    case STATE_SUCCEEDED:
-      return &m_resultOrException;
-    case STATE_FAILED:
-      Object e(m_resultOrException.m_data.pobj);
-      throw e;
+  assert(!isFinished());
+  assert(!session->isInContext() || session->getCurrentContext()->isRunning());
+
+  // enter new asio context and set up guard that will exit once we are done
+  session->enterContext();
+
+  assert(session->isInContext());
+  assert(!session->getCurrentContext()->isRunning());
+
+  try {
+    // import this wait handle to the newly created context
+    // throws if cross-context cycle found
+    enterContext(session->getCurrentContextIdx());
+
+    // run queues until we are finished
+    session->getCurrentContext()->runUntil(this);
+  } catch (Object exception) {
+    // recover from PHP exceptions; HPHP internal exceptions are deliberately
+    // ignored as there is no easy way to recover from them
+    session->exitContext();
+    throw;
   }
+  session->exitContext();
 
-  throw FatalErrorException(
-      "Invariant violation: join succeeded, but wait handle not ready");
+  assert(isFinished());
 }
 
 c_WaitableWaitHandle* c_WaitableWaitHandle::getChild() {

hphp/runtime/ext/ext_asio.cpp

@@ -24,35 +24,12 @@
 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 
-namespace {
-  const context_idx_t MAX_CONTEXT_DEPTH = std::numeric_limits<context_idx_t>::max();
-}
-
 void f_asio_enter_context() {
-  AsioSession* session = AsioSession::Get();
-  if (UNLIKELY(session->getCurrentContextIdx() >= MAX_CONTEXT_DEPTH)) {
-    Object e(SystemLib::AllocInvalidOperationExceptionObject(
-      "Unable to enter asio context: too many contexts open"));
-    throw e;
-  }
-
-  session->enterContext();
+  // TODO: remove from API
 }
 
 void f_asio_exit_context() {
-  AsioSession* session = AsioSession::Get();
-  if (UNLIKELY(!session->isInContext())) {
-    Object e(SystemLib::AllocInvalidOperationExceptionObject(
-      "Unable to exit asio context: not in a context"));
-    throw e;
-  }
-  if (UNLIKELY(session->getCurrentContext()->isRunning())) {
-    Object e(SystemLib::AllocInvalidOperationExceptionObject(
-      "Unable to exit asio context: a continuation is running"));
-    throw e;
-  }
-
-  session->exitContext();
+  // TODO: remove from API
 }
 
 Object f_asio_get_current() {

hphp/runtime/ext/ext_asio.h

@@ -87,8 +87,6 @@ class c_WaitHandle : public ExtObjectData {
   virtual String getName() = 0;
 
  protected:
-  virtual const TypedValue* join() = 0;
-
   uint8_t getState() { return o_subclassData.u8[0]; }
   void setState(uint8_t state) { o_subclassData.u8[0] = state; }
 
@@ -142,9 +140,6 @@ class c_StaticResultWaitHandle : public c_StaticWaitHandle {
 
  public:
   String getName();
-
- protected:
-  const TypedValue* join();
 };
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -170,12 +165,6 @@ class c_StaticExceptionWaitHandle : public c_StaticWaitHandle {
 
  public:
   String getName();
-
- protected:
-  const TypedValue* join();
-
- private:
-  Object m_exception;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -207,6 +196,7 @@ class c_WaitableWaitHandle : public c_WaitHandle {
   c_BlockableWaitHandle* addParent(c_BlockableWaitHandle* parent);
 
   virtual void enterContext(context_idx_t ctx_idx) = 0;
+  void join();
 
  protected:
   void setResult(const TypedValue* result);
@@ -220,8 +210,6 @@ class c_WaitableWaitHandle : public c_WaitHandle {
   c_BlockableWaitHandle* getFirstParent() { return m_firstParent; }
   c_BlockableWaitHandle* getParentInContext(context_idx_t ctx_idx);
 
-  const TypedValue* join();
-
   virtual c_WaitableWaitHandle* getChild();
   bool hasCycle(c_WaitableWaitHandle* start);