Permalink
Browse files

HHVM Debugger: Fix hang in debugger extension shutdown

Summary:
This fixes a race preventing the debugger extension from shutting down cleanly.

The session cleanup thread deletes sessions in an async job because the session thread can be blocked running user code (its the HPHP request backing the console REPL). It therefore can block indefinitely if the user has called into a native method or has an infinite loop etc, and so we can't wait to join to it on the thread that needs to communicate with the debugger client.

If this thread is cleaning up sessions due to the client disconnecting, and the extension shutdown thread signals m_sessionCleanupCondition before the cleanup thread re-acquires the lock, we can erroneously go to sleep again because we're failing to re-check m_sessionCleanupTerminating after re-acquiring the lock but before sleeping on the condition variable.

Reviewed By: alexeyt

Differential Revision: D9038429

fbshipit-source-id: 67fe04876b0f4845cb5ff9418d301cf192ddcc75
  • Loading branch information...
ebluestein authored and fredemmott committed Jul 31, 2018
1 parent 62a3095 commit a8fd6b8d1c47d24f8cbbfe887642c45688c418d3
Showing with 9 additions and 1 deletion.
  1. +9 −1 hphp/runtime/ext/vsdebug/debugger.cpp
@@ -46,10 +46,18 @@ void Debugger::runSessionCleanupThread() {
while (true) {
{
std::unique_lock<std::mutex> lock(m_sessionCleanupLock);
m_sessionCleanupCondition.wait(lock);
// Read m_sessionCleanupTerminating while the lock is held.
terminating = m_sessionCleanupTerminating;
if (!terminating) {
// Wait for signal
m_sessionCleanupCondition.wait(lock);
// Re-check terminating flag while the lock is still re-acquired.
terminating = m_sessionCleanupTerminating;
}
// Make a local copy of the session pointers to delete and drop
// the lock.
sessionsToDelete = m_cleanupSessions;

0 comments on commit a8fd6b8

Please sign in to comment.