From d6e869dfa9a2e20b22aff1a1cb9491a0fa256100 Mon Sep 17 00:00:00 2001 From: Jan Oravec Date: Sat, 15 Jun 2013 18:23:02 -0700 Subject: [PATCH] Recover from C++ exceptions in AsioExternalThreadEvent::unserialize(), part1 AsioExternalThreadEvent::unserialize() may legally throw PHP exceptions. When that happens, construction of PHP exceptions reenters VM, surprised flag is checked and pending C++ exceptions may be thrown. Let's make sure the event is properly destroyed. If it is not, the web request will get stuck forever waiting for the finished event to finish. --- .../external_thread_event_wait_handle.cpp | 41 ++++++++++++------- hphp/runtime/ext/ext_asio.h | 1 + 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/hphp/runtime/ext/asio/external_thread_event_wait_handle.cpp b/hphp/runtime/ext/asio/external_thread_event_wait_handle.cpp index d476a4373..b71551de4 100644 --- a/hphp/runtime/ext/asio/external_thread_event_wait_handle.cpp +++ b/hphp/runtime/ext/asio/external_thread_event_wait_handle.cpp @@ -80,6 +80,19 @@ void c_ExternalThreadEventWaitHandle::initialize(AsioExternalThreadEvent* event, } } +void c_ExternalThreadEventWaitHandle::destroyEvent() { + // destroy event and its private data + m_event->release(); + m_event = nullptr; + m_privData = nullptr; + + // unregister from sweep() + unregister(); + + // drop ownership by pending event (see initialize()) + decRefObj(this); +} + void c_ExternalThreadEventWaitHandle::abandon(bool sweeping) { assert(getState() == STATE_WAITING); assert(getCount() == 1 || sweeping); @@ -88,11 +101,8 @@ void c_ExternalThreadEventWaitHandle::abandon(bool sweeping) { getContext()->unregisterExternalThreadEvent(m_index); } - // event is abandoned, destroy it, unregister sweepable and decref ownership - m_event->release(); - m_event = nullptr; - unregister(); - decRefObj(this); + // clean up + destroyEvent(); } void c_ExternalThreadEventWaitHandle::process() { @@ -102,22 +112,23 @@ void c_ExternalThreadEventWaitHandle::process() { getContext()->unregisterExternalThreadEvent(m_index); } + // clean up once event is processed + auto exit_guard = folly::makeGuard([&] { destroyEvent(); }); + + TypedValue result; try { - TypedValue result; m_event->unserialize(&result); - assert(tvIsPlausible(&result)); - setResult(&result); - tvRefcountedDecRefCell(&result); } catch (const Object& exception) { setException(exception.get()); + return; + } catch (...) { + setException(AsioSession::Get()->getAbruptInterruptException().get()); + throw; } - // event is processed, destroy it, unregister sweepable and decref ownership - m_event->release(); - m_event = nullptr; - m_privData = nullptr; - unregister(); - decRefObj(this); + assert(tvIsPlausible(&result)); + setResult(&result); + tvRefcountedDecRefCell(&result); } String c_ExternalThreadEventWaitHandle::getName() { diff --git a/hphp/runtime/ext/ext_asio.h b/hphp/runtime/ext/ext_asio.h index f2efcb221..6810cd782 100644 --- a/hphp/runtime/ext/ext_asio.h +++ b/hphp/runtime/ext/ext_asio.h @@ -493,6 +493,7 @@ class c_ExternalThreadEventWaitHandle : public c_WaitableWaitHandle, public Swee private: void initialize(AsioExternalThreadEvent* event, ObjectData* priv_data); + void destroyEvent(); c_ExternalThreadEventWaitHandle* m_nextToProcess; AsioExternalThreadEvent* m_event;